Incorrect price calculation for first token of each generation due to delayed generation increment

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

After incrementing each generation, the priceIncrement is incremented by priceIncrementByGen in _incrementGeneration. The generation incrementation occurs when minting the first token of the new generation, not when minting the last token of the current generation. This leads to the issue that when minting the first token of each new generation, the priceIncrementByGen is not included in the price because calculateMintPrice is called before the generation is incremented:

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

@>    _mintInternal(msg.sender, mintPrice); // @audit minting token and incrementing generation if necessary

    uint256 excessPayment = msg.value - mintPrice;
    if (excessPayment > 0) {
      (bool refundSuccess, ) = msg.sender.call{ value: excessPayment }('');
      require(refundSuccess, 'Refund of excess payment failed.');
    }
  }

When minting the first token of a new generation, the current generation is incremented after calculating the minting price because the increment occurs inside _mintInternal:

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

As a result, the required mint price for the first token of each new generation does not include the priceIncrementByGen.

Tools Used

Manual Review

Recommended Mitigation Steps

The root cause of this issue is that the generation incrementation occurs when minting the first token of the new generation, rather than when minting the last token of the current generation. A simple fix is to move the generation incrementation check to the end of the mint process. Below is a suggestion for updating the code in mintToken and _mintInternal

// mintToken

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);

    uint256 excessPayment = msg.value - mintPrice;
    if (excessPayment > 0) {
      (bool refundSuccess, ) = msg.sender.call{ value: excessPayment }('');
      require(refundSuccess, 'Refund of excess payment failed.');
    }

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

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

Assessed type

Context

[c4-judge]

koolexcrypto marked the issue as duplicate of #564