Wrong minting logic based on total token count across generations

[howlbot-integration]

Lines of code

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

Vulnerability details

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

Summary

TraitForgeNft::mintWithBudget function is similar to mintToken, but allows users to mint multiple tokens in a single transaction if they have a budget exceeding the minting price for one token.
However, _tokenIds tracks the total number of tokens ever minted, not just the tokens in the current generation.

Impact

In the current implementation, _tokenIds is used to control the minting process. The check while (budgetLeft >= mintPrice && _tokenIds < maxTokensPerGen) ensures that minting will stop when current generation minted tokens reaches maxTokensPerGen.
Instead of checking the number of tokens minted in the current generation, the function incorrectly checks the total number of tokens minted across all generations (_tokenIds).

Proof of Concept

Here is the current implementation of the mintWithBudget function in the smart contract on line 215:

TraitForgeNft.sol
202:   function mintWithBudget(
203:     bytes32[] calldata proof
204:   )
205:     public
206:     payable
207:     whenNotPaused
208:     nonReentrant
209:     onlyWhitelisted(proof, keccak256(abi.encodePacked(msg.sender)))
210:   {
211:     uint256 mintPrice = calculateMintPrice();
212:     uint256 amountMinted = 0;
213:     uint256 budgetLeft = msg.value;
214: 
215:     while (budgetLeft >= mintPrice && _tokenIds < maxTokensPerGen) { // @audit - _tokenIds counts total tokens minted, not just current generation
216:       _mintInternal(msg.sender, mintPrice);
217:       amountMinted++;
218:       budgetLeft -= mintPrice;
219:       mintPrice = calculateMintPrice();
220:     }
221:     if (budgetLeft > 0) {
222:       (bool refundSuccess, ) = msg.sender.call{ value: budgetLeft }('');
223:       require(refundSuccess, 'Refund failed.');
224:     }
225:   }

The function will not allow minting if _tokenIds is greater than 10,000 which will happen after the 1st generation is fully minted.

Tools Used

Manual Review

Recommended Mitigation Steps

Use generationMintCounts[currentGeneration] instead of _tokenIds.

- while (budgetLeft >= mintPrice && _tokenIds < maxTokensPerGen) {
+ while (budgetLeft >= mintPrice && generationMintCounts[currentGeneration] <= maxTokensPerGen) {

Assessed type

Invalid Validation

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[AtanasDimulski]

@koolexcrypto thanks for the swift judging! I have the same finding in my QA report #32 L-01, I guess it has been missed. Can you please make it a duplicate of this issue? Thanks for your time!