Incorrect mintPrice When maxTokensPerGen Is Reached

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

In the white paper, it is expected that **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, the current implementation of the calculateMintPrice function does not correctly account for generation increments happening after price calculation, leading to incorrect pricing. This could result in users either facing DoS (Denial of Service) issues or overpaying for minting.

Proof of Concept

In the white paper, it is expected that **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.**.

The calculateMintPrice function calculates the mint price based on the current generation and the number of mints within that generation:

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

However, the function is called before the generation is incremented in the mintToken function:

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

@=> _mintInternal(msg.sender, mintPrice); // <= generation increment

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

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

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

In the edge case where generationMintCounts[currentGeneration] equals maxTokensPerGen, the generation should increase by 1, and the price should be reset to the startPrice. However, the calculateMintPrice function calculates the price before this increment, leading to an overestimation of the price.

Example Scenario

1. currentGeneration = 1 and generationMintCounts[currentGeneration] = maxTokensPerGen = 10000.
2. User Bob calls mintToken. The calculated mintPrice is startPrice + generationMintCounts[currentGeneration] * priceIncrement = 0.005 + 10000 * 0.0000245 = 0.25 ETH.
3. However, this NFT should be the first of the second generation, and the correct price should be startPrice = 0.005 ETH.
4. Bob either cannot mint if he sends 0.005 ETH (transaction reverts) or overpays significantly if he sends 0.25 ETH.

Note: Since this is also used in mintWithBudget, this could also lead to the while loop exits earlier than expected.

Tools Used

Manual

Recommended Mitigation Steps

To mitigate this issue:

• Check and Update Generation at the Start of Minting: Move the generation check and update logic to the start of the mintToken function to ensure the correct generation and price are calculated before minting.
• Call _incrementGeneration after each mint: update the _mintInternal function just like _mintNewEntity to include a generationMintCounts[gen] check and update the generation if necessary.

Assessed type

Payable

[c4-judge]

koolexcrypto marked the issue as duplicate of #564