Last-second minter would overpay mint price if salesOption == 2

[c4-submissions]

Lines of code

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

Vulnerability details

NextGenMinterContract#getPrice returns the price for minting a new token. If the collection has salesOption with value 2 and the current timestamp is between allowlistStartTime and publicEndTime, this function would calculate the mint price between values of collectionMintCost and collectionEndMintCost. Price is changing consistently with the growing block.timestamp value:

    function getPrice(uint256 _collectionId) public view returns (uint256) {
        uint tDiff;
        if (collectionPhases[_collectionId].salesOption == 3) {
            // increase minting price by mintcost / collectionPhases[_collectionId].rate every mint (1mint/period)
            // to get the price rate needs to be set
            if (collectionPhases[_collectionId].rate > 0) {
                return collectionPhases[_collectionId].collectionMintCost + ((collectionPhases[_collectionId].collectionMintCost / collectionPhases[_collectionId].rate) * gencore.viewCirSupply(_collectionId));
            } else {
                return collectionPhases[_collectionId].collectionMintCost;
            }
        } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
            // decreases exponentially every time period
            // collectionPhases[_collectionId].timePeriod sets the time period for decreasing the mintcost
            // if just public mint set the publicStartTime = allowlistStartTime
            // if rate = 0 exponetialy decrease
            // if rate is set the linear decrase each period per rate
            tDiff = (block.timestamp - collectionPhases[_collectionId].allowlistStartTime) / collectionPhases[_collectionId].timePeriod;
            uint256 price;
            uint256 decreaserate;
            if (collectionPhases[_collectionId].rate == 0) {
                price = collectionPhases[_collectionId].collectionMintCost / (tDiff + 1);
                decreaserate = ((price - (collectionPhases[_collectionId].collectionMintCost / (tDiff + 2))) / collectionPhases[_collectionId].timePeriod) * ((block.timestamp - (tDiff * collectionPhases[_collectionId].timePeriod) - collectionPhases[_collectionId].allowlistStartTime));
            } else {
                if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
                    price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
                } else {
                    price = collectionPhases[_collectionId].collectionEndMintCost;
                }
            }
            if (price - decreaserate > collectionPhases[_collectionId].collectionEndMintCost) {
                return price - decreaserate; 
            } else {
                return collectionPhases[_collectionId].collectionEndMintCost;
            }
        } else {
            // fixed price
            return collectionPhases[_collectionId].collectionMintCost;
        }
    }

Minting functions in MinterContract allow minting tokens when block.timestamp == publicEndTime:

File: MinterContract.sol
196:     function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
...
221:         } else if (block.timestamp >= collectionPhases[col].publicStartTime && block.timestamp <= collectionPhases[col].publicEndTime) {
222:             phase = 2;
223:             require(_numberOfTokens <= gencore.viewMaxAllowance(col), "Change no of tokens");
224:             require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");
225:             mintingAddress = msg.sender;
226:             tokData = '"public"';

At the same time getPrice function returns the collectionMintCost price in case block.timestamp is equal to publicEndTime. This creates a scenario where minting at the publicEndTime - 1 timestamp is more cost-effective than minting at the publicEndTime timestamp. This discrepancy arises from the accurate price drop in the first case contrasted with the wrongly returned collectionMintCost value in the second case.

Impact

Last-second minter would overpay the mint price if the case of the collection with salesOption == 2.

Proof of Concept

The next foundry test shows how last-second minter is overpaid compared to the minter from the previous second:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;

import "./Base.t.sol";
import "forge-std/Test.sol";

contract OverpayOnLastMint is Base, Test {
    
    address user1;
    address user2;

    function setUp() public {
        user1 = makeAddr("user1");
        user2 = makeAddr("user2");
        _deploy();
        skip(1 days);
        _createCollectionWithSaleOption2();
    }

    function test_exploit() public payable {
        uint256 collectionId = 1;
        bytes32[] memory emptyProof;
        uint256 endTime = nextGenMinterContract.getEndTime(collectionId);
        vm.warp(endTime-1);

        // Minting price for one second before minting ends is correctly dropped lower 
        uint256 price = nextGenMinterContract.getPrice(collectionId);
        vm.deal(user1, price);
        vm.prank(user1);
        nextGenMinterContract.mint{value: price}(collectionId, 1, 1, "", user1, emptyProof, address(0), 0);
        emit log_uint(price);
        
        // Minting when the timestamp is equal to publicEndTime would result in a higher mint price 
        skip(1);
        price = nextGenMinterContract.getPrice(collectionId);
        vm.deal(user2, price);
        vm.prank(user2);
        nextGenMinterContract.mint{value: price}(collectionId, 1, 1, "", user2, emptyProof, address(0), 0);
        emit log_uint(price);
    }
}

Recommended Mitigation Steps

Consider updating the next line in MinterContract.sol:

File: MinterContract.sol
-         } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
+         } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp <= collectionPhases[_collectionId].publicEndTime){

Assessed type

Math

[c4-pre-sort]

141345 marked the issue as duplicate of #1391

[c4-judge]

alex-ppg marked the issue as satisfactory