Incorrect calculation of the price of each new generation first NFT

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

The TraitForge protocol supports multiple generations, (10 at the beginning) and each generation can have 10_000 NFTs(this value is hardcoded in the TraitForgeNft cotnract. The price of each NFT is calculated in the calculateMintPrice() function. The first NFT costs 0.005 ETH this is the startPrice and each subsequent one rises linearly by 0.0000245 ETH this is the priceIncrement. After 10_000 NFTs are minted the priceIncrement is increased by the priceIncrementByGen which is 0.000005 ETH. The second generation NFTs cost should start from 0.005 ETH again, and increase linearly this time by 0.0000245 + 0.000005 = 0.0000295 ETH. As described in the docs: 10,000 “Gen 1” entities are available to mint. The first starts at 0.005 ETH and each subsequent one rises linearly by 0.0000245 ETH until the final is 0.25 ETH. In total if all are minted then 1,275 ETH is raised in Generation 1. Each generation’s price increment increases by 0.000005. However this is not the case. As we can see in the mintToken() function:

  function mintToken(bytes32[] calldata proof ) public payable whenNotPaused nonReentrant onlyWhitelisted(proof, keccak256(abi.encodePacked(msg.sender))) {
    uint256 mintPrice = calculateMintPrice();
    require(msg.value >= mintPrice, 'Insufficient ETH send for minting.');
    _mintInternal(msg.sender, mintPrice);
    ...
  }

First calculateMintPrice() function is called to calculate the mint price:

  function calculateMintPrice() public view returns (uint256) {
    uint256 currentGenMintCount = generationMintCounts[currentGeneration];
    uint256 priceIncrease = priceIncrement * currentGenMintCount;
    uint256 price = startPrice + priceIncrease;
    return price;
  }

Then_mintInternal() function is called where it is checked whether the generation should be increased or not, and then the NFT is minted.

  function _mintInternal(address to, uint256 mintPrice) internal {
    if (generationMintCounts[currentGeneration] >= maxTokensPerGen) {
      _incrementGeneration();
    }
    ...
  }

When users have minted 10_000 NFTs of the first generation, when the next user tries to mint the 1st NFT of the second generation, the calculateMintPrice() function will calculate the mint price as follows:

• priceIncrease = 0.0000245e18 * 10_000 = 0.245e18
• price = 0.005e18 + 0.245e18 = 0.25e18
  Thus the user will have to pay 0.25 ETH for the first NFT of the second generation instead of 0.005 ETH, which is 50 times more expensive. This will occur for each first NFT of a generation, whether users have forged NFTs previously doesn't matter. Currently the protocol has 10 generations, however they have mentioned in the docs that they intend to increase this number, as the protocol progresses. This miscalculation may defer users from buying the first NFT of the second generation, and thus only the first generation of NFTs will be minted which results in a loss for the protocol, as part of the funds are distributed to the devs and owner.

Proof of Concept

Gist

After completing the steps in the above mentioned gist add the following test to the AuditorTests.t.sol file:

    function test_WrongPriceCalculation() public {
        /// @notice skip the whitelist period
        skip(86401);

        vm.startPrank(alice);
        deal(alice, 10_000e18);
        bytes32[] memory fakeProof = new bytes32[](1);
        fakeProof[0] = keccak256(abi.encodePacked(uint256(1)));
        for(uint256 i; i < 10_000; i++) {
            traitForgeNft.mintToken{value: 0.25e18}(fakeProof);
        }
        console2.log("The amount of minted NFTs for generation 1: ", traitForgeNft.generationMintCounts(1));
        console2.log("The amount of minted NFTs for generation 2: ", traitForgeNft.generationMintCounts(2));
        console2.log("The current generation: ", traitForgeNft.currentGeneration());
        vm.stopPrank();

        vm.startPrank(bob);
        console2.log("Next Mint price: ", traitForgeNft.calculateMintPrice());
        uint256 firstNFTGenerationTwoPrice = 0.25e18;
        deal(bob, firstNFTGenerationTwoPrice);
        traitForgeNft.mintToken{value: firstNFTGenerationTwoPrice}(fakeProof);
        console2.log("Bob's balance after he mints the next NFT: ", bob.balance);
        console2.log("The amount of minted NFTs for generation 2: ", traitForgeNft.generationMintCounts(2));
        console2.log("The current generation: ", traitForgeNft.currentGeneration());
        console2.log("Token generation of token 10_001: ", traitForgeNft.tokenGenerations(10_001));
        vm.assertEq(bob, traitForgeNft.initialOwners(10_001));
        vm.stopPrank();

        vm.startPrank(tom);
        console2.log("Next Mint price: ", traitForgeNft.calculateMintPrice());
        uint256 secondNFTGenerationTwoPrice = 5029500000000000;
        deal(tom, secondNFTGenerationTwoPrice);
        traitForgeNft.mintToken{value: secondNFTGenerationTwoPrice}(fakeProof);
        console2.log("Tom's balance after he mints the next NFT: ", tom.balance);
        vm.assertGt(firstNFTGenerationTwoPrice, secondNFTGenerationTwoPrice);
        vm.stopPrank();
    }

Logs:
  The amount of minted NFTs for generation 1:  10000
  The amount of minted NFTs for generation 2:  0
  The amount of minted NFTs for generation 1:  10000
  The amount of minted NFTs for generation 2:  0
  The current generation:  1
  The current generation:  1
  Next Mint price:  250000000000000000
  Next Mint price:  250000000000000000
  Bob's balance after he mints the next NFT:  0
  Bob's balance after he mints the next NFT:  0
  The amount of minted NFTs for generation 2:  1
  The current generation:  2
  The amount of minted NFTs for generation 2:  1
  The current generation:  2
  Token generation of token 10_001:  2
  Next Mint price:  5029500000000000
  Tom's balance after he mints the next NFT:  0

To run the test use: forge test -vvv --mt test_WrongPriceCalculation

Tools Used

Manual review & Foundry

Recommended Mitigation Steps

Consider calculating the price of an NFT in the _mintInternal() function after the check for whether a generation should be increased or not is performed:

    if (generationMintCounts[currentGeneration] >= maxTokensPerGen) {
      _incrementGeneration();
    }

Assessed type

Context

[c4-judge]

koolexcrypto marked the issue as duplicate of #564