Incorrect price calculation for edge case

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

According to the calculateMintPrice(), the first NFT in each generation will be minted with price startPrice. But the actual price for first NFT in each generation is startPrice + priceIncrement * maxTokensPerGen. The miner has to pay more funds than expected.

Proof of Concept

The same vulnerability happens in mintToken() and mintWithBudget(). Let's take mintToken() as one example:
The first NFT in generation 0 price: startPrice
The second NFT in generation 0 price: startPrice + priceIncrement
...
The 100000th NFT in generation 0 price: startPrice + priceIncrement * 99999
The next one should be the first NFT in generation 2, expected price should be startPrice, but actual price is startPrice + priceIncrement * 100000

The vulnerability is that we calculate this new NFT's price and then we calculated which generation this NFT belongs to.
We should calculate this newly NFT's generation, and then calculate this NFT's price.

  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);
    ...
   }
  function calculateMintPrice() public view returns (uint256) {
    uint256 currentGenMintCount = generationMintCounts[currentGeneration];
    uint256 priceIncrease = priceIncrement * currentGenMintCount;
    uint256 price = startPrice + priceIncrease;
    return price;
  }
  function _mintInternal(address to, uint256 mintPrice) internal {
    if (generationMintCounts[currentGeneration] >= maxTokensPerGen) {
      _incrementGeneration();
    }
    _tokenIds++;
    uint256 newItemId = _tokenIds;
    _mint(to, newItemId);
    uint256 entropyValue = entropyGenerator.getNextEntropy();

    tokenCreationTimestamps[newItemId] = block.timestamp;
    tokenEntropy[newItemId] = entropyValue;
    tokenGenerations[newItemId] = currentGeneration;
    // This generation's mint count.
    generationMintCounts[currentGeneration]++;
    initialOwners[newItemId] = to;
    if (!airdropContract.airdropStarted()) {
      airdropContract.addUserAmount(to, entropyValue);
    }
    ...
}

Tools Used

Manual

Recommended Mitigation Steps

Calculate newly NFT's generation and then calculate newly NFT's price.

Assessed type

Error

[c4-judge]

koolexcrypto marked the issue as duplicate of #564