TraitForgeNft contract cannot increment generations due to the wrong modifier in EntropyGenerator

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntropyGenerator/EntropyGenerator.sol#L206-L216 https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/TraitForgeNft/TraitForgeNft.sol#L353

Vulnerability details

Impact

EntropyGenerator.sol contract sets the allowed caller variable at the time of deployment. This is supposed to be TraitForgeNft.sol contract so that different functions can be called from it. However, the function initializeAlphaIndices() has onlyOwner modifier from Ownable, unlike other functions which have onlyAllowedCaller modifier. This will cause many functions in TraitForgeNft.sol to revert when the function _incrementGeneration() is called (such as mintToken(), mintWithBudget() or forge()) causing a denial of service. The _incrementGeneration() will get called always when there is a need to switch from one generation to another, i.e. when the maximum number of tokens per generation is minted. This means, that users will be able to mint only the first generation.

It is confirmed by the sponsor that the Multisig wallet is supposed to be the owner of all contracts, except from the Airdrop.sol contract (this one is supposed to be owned by TraitForgeNft.sol contract).

Proof of Concept

For the sake of simplicity add this function to the TraitForgeNft.sol contract

 function _testCall() public {
    entropyGenerator.initializeAlphaIndices();
  }

And add this test to the TraitForgeNft.test.ts

it('should not be possible to initializeAlphaIndices() if not owner', async() => {
      await expect(nft._testCall()).to.be.reverted;
    });

Tools Used

VS Code, Hardhat, Manual Review

Recommended Mitigation Steps

Use onlyAllowedCaller modifier instead of onlyOwner on initializeAlphaIndices() so that TraitForgeNft.sol is allowed to call it.

Assessed type

DoS

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)