Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L540-L564

Vulnerability details

Impact

When a collection is using linear descending sale model the last period will return lower price than expected. When setCollectionCosts() is set with the following parameters for example:

  nextGenMinterContract.setCollectionCosts(
          1, // _collectionID
          4.1e18, // _collectionMintCost 4.1 eth - Starting Price
          3e18, // _collectionEndMintCost 3 eth - Resting Price
          0.2e18, // _rate
          600, // _timePeriod
          2, // _salesOptions
          delAddress // delAddress
        );

The problem comes in the following line: if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) . In Solidity, division rounds towards zero. As shown in the example parameters above if collectionMintCost is 4.1e18, collectionEndMintCost is 3e18 and the rate is 0.2e18, the left part of the if statement will try to divide 1.1e18 with 0.2e18 resulting in 5. Accordign to the docs: At each time period (which can vary), the minting cost decreases linearly until it reaches its resting price (ending minting cost) and stays there until the end of the minting phase. When the left side of the if statement is compared to tDiff, which shows how many periods have passed since the start of the sale in the last period of the sale, the check won't pass and will directly return the collectionEndMintCost price, which is lower than the expected price for the period. When a user pays less for an NFT the MinterContract.sol receives less ETH than it is expected by the artist of the collection and the team. As the payArtist() function distributes the funds generated by a certain collection to addresses supplied by the artist and the team of nextgen. This results in a loss for both the artist of the collection and the nextgen team. The supplied values are not a single magic combination that leads to this issue, there are a lot of possible combinations. As stated in the docs a period can be a minute, hour or a day. In the scenario where users are allowed to mint NFTs for a day on a discounted price the losses for the protocol will be big. This can happen in a lot of collections utilizing the linear descending sale model thus the high severity. In the below POC the parameters set in setCollectionCosts() are chosen for simplicity and close representation of the parameters provided in the docs for the linear descending sale model. As can be seen from the Logs the price in the 6th period is supposed to be 3.1e18 but in realty is 3e18 .

Note: This is a completely different issue than the known issue presented by the team in the docs as this issue pertains to the linear descending sale model and is not concerned with a low minting cost: We are aware of the price rounding errors when the Exponential Descending Sales Model is used and the minting cost is low, thus any finding on this is not valid.

Proof of Concept

To run the POC first follow the steps in this link: https://hardhat.org/hardhat-runner/docs/advanced/hardhat-and-foundry in order to add foundry to the project.

Create a AuditorPriceTests.t.sol file in the test folder and add the following to it:

pragma solidity 0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol"; 
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";
import {ReentrancyAuctionDemo} from "./ReentrancyAuctionDemo.sol";

contract AuditorPriceTests  is Test {
    DelegationManagementContract public delegationManagementContract;
    randomPool public rPool;
    NextGenAdmins public nextGenAdmins;
    NextGenCore public nextGenCore;
    NextGenMinterContract public nextGenMinterContract;
    NextGenRandomizerNXT public nextGenRandomizerNXT;
    auctionDemo public aDemo;
    ReentrancyAuctionDemo public reentrancyAuctionDemo;

    address public contractOwner = vm.addr(1);
    address public alice = vm.addr(2);
    address public bob = vm.addr(3);
    address public hacker = vm.addr(4);
    address public globalAdmin = vm.addr(5);
    address public collectionAdmin = vm.addr(6);
    address public functionAdmin = vm.addr(7);
    address public john = vm.addr(8);
    address public auctionNftOwner = vm.addr(9);
    address public tom = vm.addr(10);
    address public delAddress = address(0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B);
    bytes32 public merkleRoot = 0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870;

    function setUp() public {
        vm.startPrank(contractOwner);
        /// INFO: Deploy contracts
        delegationManagementContract = new DelegationManagementContract();
        rPool = new randomPool();
        nextGenAdmins = new NextGenAdmins();
        nextGenCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(nextGenAdmins));
        nextGenMinterContract = new NextGenMinterContract(address(nextGenCore), address(delegationManagementContract), address(nextGenAdmins));
        nextGenRandomizerNXT = new NextGenRandomizerNXT(address(rPool), address(nextGenAdmins), address(nextGenCore));

        /// INFO: Set admins
        nextGenAdmins.registerAdmin(globalAdmin, true);
        nextGenAdmins.registerCollectionAdmin(1, collectionAdmin, true);
        nextGenAdmins.registerCollectionAdmin(2, collectionAdmin, true);
        vm.stopPrank();

        /// INFO: Set up collection in genCore
        vm.startPrank(globalAdmin);
        string[] memory collectionScript = new string[](1);
        collectionScript[0] = "desc";
        nextGenCore.createCollection("Test Collection 1", "Artist 1", "For testing", "www.test.com", "CCO", "https://ipfs.io/ipfs/hash/", "", collectionScript);
        nextGenCore.addRandomizer(1, address(nextGenRandomizerNXT));
        nextGenCore.addMinterContract(address(nextGenMinterContract));
        vm.stopPrank();

        /// INFO: Set up collection params in minter contract
        vm.startPrank(collectionAdmin);
        /// INFO: Set up collection 1
        nextGenCore.setCollectionData(1, collectionAdmin, 2, 10000, 1000);
        nextGenMinterContract.setCollectionCosts(
          1, // _collectionID
          4.1e18, // _collectionMintCost 4.1 eth - Starting Price
          3e18, // _collectionEndMintCost 3 eth - Resting Price
          0.2e18, // _rate
          600, // _timePeriod
          2, // _salesOptions
          delAddress // delAddress
        );

        nextGenMinterContract.setCollectionPhases(
          1, // _collectionID
          600, // _allowlistStartTime
          600, // _allowlistEndTime
          600, // _publicStartTime
          6000, // _publicEndTime
          merkleRoot // _merkleRoot
        );

        /// INFO: Deploy AuctionDemo contract and approve contract to transfer NFTs
        vm.startPrank(auctionNftOwner);
        aDemo = new auctionDemo(address(nextGenMinterContract), address(nextGenCore), address(nextGenAdmins));
        nextGenCore.setApprovalForAll(address(aDemo), true);
        vm.stopPrank();
    }

    function test_RoundingDownInLinearDescending() public {
        skip(600);
        console.log("This is the current block timestamp: ", block.timestamp);
        console.log("Get the price to mint for a collection with linear descending: ", nextGenMinterContract.getPrice(1));
        skip(600);
        console.log("This is the current block timestamp: ", block.timestamp);
        console.log("Get the price to mint for a collection with linear descending: ", nextGenMinterContract.getPrice(1));
        skip(600);
        console.log("This is the current block timestamp: ", block.timestamp);
        console.log("Get the price to mint for a collection with linear descending: ", nextGenMinterContract.getPrice(1));
        skip(600);
        console.log("This is the current block timestamp: ", block.timestamp);
        console.log("Get the price to mint for a collection with linear descending: ", nextGenMinterContract.getPrice(1));
        skip(600);
        console.log("This is the current block timestamp: ", block.timestamp);
        console.log("Get the price to mint for a collection with linear descending: ", nextGenMinterContract.getPrice(1));
        skip(600);
        console.log("This is the current block timestamp: ", block.timestamp);
        console.log("Get the price to mint for a collection with linear descending: ", nextGenMinterContract.getPrice(1));

        /// INFO: Alice mints an NFT cheaper than she should resulting in the protocol loosing money
        vm.startPrank(alice);
        vm.deal(alice, 3e18);
        bytes32[] memory merkleProof = new bytes32[](1);
        merkleProof[0] = merkleRoot;
        nextGenMinterContract.mint{value: 3e18}(1, 1, 1, "{'tdh': '100'}", alice, merkleProof, alice, 2);
        assertEq(alice.balance, 0);
        assertEq(alice, nextGenCore.ownerOf(10_000_000_000));
        vm.stopPrank();
    }
}

Logs:
  This is the current block timestamp:  601
  Get the price to mint for a collection with linear descending:  4100000000000000000
  This is the current block timestamp:  1201
  Get the price to mint for a collection with linear descending:  3900000000000000000
  This is the current block timestamp:  1801
  Get the price to mint for a collection with linear descending:  3700000000000000000
  This is the current block timestamp:  2401
  Get the price to mint for a collection with linear descending:  3500000000000000000
  This is the current block timestamp:  3001
  Get the price to mint for a collection with linear descending:  3300000000000000000
  This is the current block timestamp:  3601
  Get the price to mint for a collection with linear descending:  3000000000000000000

To run the test use the following command: forge test -vvv --mt test_RoundingDownInLinearDescending

Tools Used

Manual Review & Foundry

Recommended Mitigation Steps

Consider rounding up the results of the left side of the if statement if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) or using => instead of >.

Assessed type

Math

code-423n4 / 2023-10-nextgen-findings

Rounding down in getPrice() linear descending sale model leads to a loss of funds #1528