search

code-423n4 / 2024-07-traitforge-findings

1 stars 0 forks source link

Incorrect Increment By Generation Calculations in `TraitForgeNft::calculateMintPrice` #17

Closed c4-bot-9 closed 2 months ago

c4-bot-9 commented 2 months ago

Lines of code

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

Vulnerability details

The price increase in TraitForgeNft::calculateMintPrice has two parts: priceIncrement, which increases the price per mint, and priceIncrementByGen, which is meant to increase price per generation. However, the current calculation is incorrect, as the per-generation increase is not flat and is also dependent on currentGenMintCount.

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

the priceIncrement is increases each time the generation is incremented using _incrementGeneration

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

As such, the existing formula for mint price is:

mintPrice = startPrice + Current Generation Increment + Increment By Generation
mintPrice = startPrice + [priceIncrement * currentGenerationMintCount] + [priceIncrementByGen * (currentGeneration - 1) * currentGenerationMintCount]

The correct formula should ensure that the generation increment is flat and does not change according to currentGenerationMintCount:

mintPrice = startPrice + [priceIncrement * currentGenerationMintCount] + [priceIncrementByGen * (currentGeneration - 1) * 10000]

Note that: The current implementation of the TraitForgeNft::calculateMintPrice function suggests that the priceIncrementByGen is incorrectly influenced by the currentGenMintCount. This behavior is unexpected because the generation increment should be flat and independent of the mint count within the current generation. Given the nature of the described issue—where the price increment calculation appears to be logically flawed—it is more likely that this is a bug rather than a deliberate design choice. The expected behavior (a flat increment per generation) is logical from an economic perspective, ensuring that each new generation of tokens has a predictable and consistent price increase. The current behavior (where the generation increment depends on the mint count) seems like it could easily result from a misunderstanding or error in implementing the intended functionality. This is further supported by the significant discrepancy between the actual and expected mint prices observed in the proof of concept.

Impact

The current implementation leads to incorrect mint price calculations, where the price does not increase as intended with each new generation. This results in significantly lower prices than expected for new token mints, causing the project to earn less revenue than planned. Over time, this could have substantial financial implications, particularly if the project relies on minting revenue for sustainability and growth. Additionally, users may perceive the pricing model as flawed or inconsistent, potentially impacting user trust and engagement with the project. Correcting this issue is crucial to ensure the economic model functions as designed, maintaining fair pricing for users and adequate revenue for the project's ongoing development and operations.

Proof of Concept

The proof can be demonstrated using hardhat-foundry library and this foundry test suite: Steps:

  1. Mint initial 10000 tokens to reach the 1st generation threshold.
  2. Mint first token in gen 2 and calculate the expected price with correct formula.
  3. Assert the expected Price > actual Price.

The results will be as such for first token in gen 2:

actualMintPrice =    5 029 500 000 000 000
expectedMintPrice = 55 024 500 000 000 000
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import '../../contracts/Airdrop/Airdrop.sol';
import '../../contracts/DAOFund/DAOFund.sol';
import '../../contracts/DevFund/DevFund.sol';
import '../../contracts/EntityForging/EntityForging.sol';
import '../../contracts/EntityTrading/EntityTrading.sol';
import '../../contracts/EntropyGenerator/EntropyGenerator.sol';
import '../../contracts/NukeFund/NukeFund.sol';
import '../../contracts/TraitForgeNft/TraitForgeNft.sol';
import '../../contracts/Trait/Trait.sol';
import '../../contracts/DAOFund/IUniswapV2Router.sol';
import { Test, console2 } from 'forge-std/Test.sol';
contract TFGNFTTest is Test {
  Airdrop public airdrop;
  DAOFund public dao;
  DevFund public dev;
  EntityForging public forge;
  EntityTrading public trade;
  EntropyGenerator public entropy;
  NukeFund public nuke;
  TraitForgeNft public nft;
  Trait public traitToken;
  IUniswapV2Router01 router;
  address userA = makeAddr('userA');
  address userB = makeAddr('userB');
  address public user1 = 0x70997970C51812dc3A010C7d01b50e0d17dc79C8;
  address public user2 = 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC;
  address public owner = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;
  function setUp() public {
    vm.roll(1); //111 for fetchListingDOS
    vm.startPrank(owner);
    traitToken = new Trait('Trait', 'TR', uint8(18), 1000000e18);
    airdrop = new Airdrop();
    router = IUniswapV2Router01(0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D);
    dao = new DAOFund(address(traitToken), address(router));
    dev = new DevFund();
    nft = new TraitForgeNft();
    forge = new EntityForging(address(nft));
    trade = new EntityTrading(address(nft));
    entropy = new EntropyGenerator(address(nft));
    nuke = new NukeFund(
      address(nft),
      address(airdrop),
      payable(address(dev)),
      payable(address(dao))
    );
    /// dao init ///
    /// dev init ///
    /// nft inits ///
    nft.setAirdropContract(address(airdrop));
    airdrop.transferOwnership(address(nft));

    entropy.writeEntropyBatch1();
    entropy.writeEntropyBatch2();
    entropy.writeEntropyBatch3();
    entropy.transferOwnership(address(nft));
    assertEq(entropy.getLastInitializedIndex(), 770);
    nft.setEntropyGenerator(address(entropy));
    nft.setEntityForgingContract(address(forge));
    nft.setNukeFundContract(payable(address(nuke)));
    trade.setNukeFundAddress(payable(address(nuke)));
    nft.setRootHash(
      0x55e8063f883b9381398d8fef6fbae371817e8e4808a33a4145b8e3cdd65e3926
    );
    vm.stopPrank();
  }
  function testMintPriceIncrementForGenIsWrong2() public {
    vm.warp(block.timestamp + 24 hours + 1);
    // mint intial tokens to reach the 1st generation treshold 
    this.mintTokens(10000); 
    uint256 balanceBefore = address(this).balance;
    // mint last token in gen 1 to see if the formula is correct. 
    this.mintTokens(1);
    uint256 mintPrice = balanceBefore - address(this).balance;
    console2.log(mintPrice); //250 000 000 000 000 000

    // mint first token in gen 2 and calculate the expected price with correct formula
    uint256 expectedPrice = nft.startPrice() +
      0.0000245 ether *
      nft.generationMintCounts(nft.currentGeneration()) +
      (nft.currentGeneration() - 1) *
      nft.priceIncrementByGen() *
      10000; //55 024 500 000 000 000

    balanceBefore = address(this).balance;
    this.mintTokens(1);
    mintPrice = balanceBefore - address(this).balance; //5 029 500 000 000 000
    // expectedPrice (correct formula)

    assert(expectedPrice > mintPrice);
  }
  ///helper functions
  function mintTokens(uint256 iterations) external {
    bytes32[] memory proof;

    for (uint256 i = 0; i < iterations; i++) {
      nft.mintToken{ value: 1000000 ether }(proof); //just a big value so it never reverts.
    }
  }
}

Tools Used

Manual Review, hardhat-foundry

Recommended Mitigation Steps

To ensure the mint price increases correctly by generation, the formula should be adjusted to apply a flat increment per generation, independent of the current generation mint count. This can be achieved by modifying the calculateMintPrice function to incorporate the generation increment correctly.

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

Assessed type

Other

c4-judge commented 1 month ago

koolexcrypto marked the issue as duplicate of #564