MinterContract.sol#mintAndAuction() reverts due to division by 0 error

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L276-L298

Vulnerability details

Bug Description

The mintAndAuction() function in MinterContract.sol has a variable tDiff that is used to check if a period has passed in order to determine if minting is allowed, MinterContract.sol#L292:

uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[_collectionID].timePeriod;

If collectionPhases[_collectionID].timePeriod is 0, which is highly likely due to this being a requirement for collections with a fixed price sales model, the transaction will revert due to a division by 0 error.

Impact

mintAndAuction() cannot be called on collections that set the _timePeriod parameter in setCollectionCosts() to 0 in MinterContract.sol, notably those with a fixed priced sales model. Projects relying on NextGen’s infrastructure that use a fixed price sales model in conjunction with auctions will be severely limited which may lead to financial loss.

Proof of Concept

Alice has 10 pieces of artwork that she would like to sell using NextGen. Her plan is to sell her NFTs using a fixed price sales model and if she does not have success in doing so she will instead put them up for auction using mintAndAuction(). She utilizes NextGen to create a collection and following the documentation regarding fixed price sales models, sets the _timePeriod to 0 when calling setCollectionCosts():

The rate and timeperiod need to be set to 0 as they do not affect the price.

When she later calls mintAndAuction() the transaction reverts due to a division by 0 error. Here is the Foundry test:

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

import {Test} from "forge-std/Test.sol";
import {stdError} from "forge-std/StdError.sol";
import {NextGenAdmins} from "../../contracts/NextGenAdmins.sol";
import {NextGenCore} from "../../contracts/NextGenCore.sol";
import {randomPool} from "../../contracts/XRandoms.sol";
import {NextGenRandomizerNXT} from "../../contracts/RandomizerNXT.sol";
import {DelegationManagementContract} from "../../contracts/NFTdelegation.sol";
import {NextGenMinterContract} from "../../contracts/MinterContract.sol";

contract MintAndAuction is Test {
    NextGenCore public nextGenCore;
    NextGenAdmins public adminsContract;
    NextGenRandomizerNXT public nextGenRandomizerNXT;
    randomPool public randomPoolContract;
    NextGenMinterContract public nextGenMinterContract;
    DelegationManagementContract public delegationManagementContract;

    address public alice;

    function setUp() public {
        // Initialize contracts
        adminsContract = new NextGenAdmins();
        nextGenCore = new NextGenCore("foo", "bar", address(adminsContract));
        randomPoolContract = new randomPool();
        nextGenRandomizerNXT = new NextGenRandomizerNXT(address(randomPoolContract), address(adminsContract), address(nextGenCore));
        delegationManagementContract = new DelegationManagementContract();
        nextGenMinterContract = new NextGenMinterContract(address(nextGenCore), address(delegationManagementContract), address(adminsContract));

        /* 
        * ======================
        * Create a collection
        * ======================
        */

        string[] memory _collectionScript = new string[](1);

        nextGenCore.createCollection(
            "foo", // _testCollectionName
            "bar", // _testArtist
            "baz", // _testDescription
            "qux", // _testWebsite
            "quux", // _testLicense
            "corge", // _testBaseURI
            "grault", // _testLibrary
            _collectionScript
        );

        address _collectionArtistAddress = makeAddr("artist");

        nextGenCore.setCollectionData(
            1, // _collectionID
            _collectionArtistAddress,
            1, // _maxCollectionPurchases
            10, // _collectionTotalSupply
            block.timestamp + 7 days // _setFinalSupplyTimeAfterMint
        );

        nextGenCore.addRandomizer(
            1, // _collectionID
            address(nextGenRandomizerNXT) // _randomizerContract
        );

        nextGenMinterContract.setCollectionCosts(
            1, // _collectionID
            1 ether, // _collectionMintCost
            1 ether, // _collectionEndMintCost
            0, // _rate
            0, // _timePeriod
            1, // _salesOption
            address(delegationManagementContract) // _delAddress
        );

        uint256 _allowlistStartTime = block.timestamp;
        uint256 _allowlistEndTime = _allowlistStartTime + 1 days;
        uint256 _publicStartTime = _allowlistEndTime + 1 days;
        uint256 _publicEndTime = _publicStartTime + 7 days;

        nextGenMinterContract.setCollectionPhases(
            1, // _collectionID
            _allowlistStartTime,
            _allowlistEndTime,
            _publicStartTime,
            _publicEndTime,
            bytes32(0) // _merkleRoot
        );

        nextGenCore.addMinterContract(address(nextGenMinterContract));

        /* 
        * ======================
        * PoC logic
        * ======================
        */

        // Set the block.timestamp to the public sale start time for simplicity
        vm.warp(_publicStartTime);

        // Create an address for Alice
        alice = makeAddr("alice");
    }

    function test_MintAndAuction() public {
        // Expecting a division by zero error
        vm.expectRevert(stdError.divisionError);

        nextGenMinterContract.mintAndAuction(
            alice, // _recipient
            "foo", // _tokenData
            123, // _saltfun_o
            1, // _collectionID
            block.timestamp + 3 days // _auctionEndTime
        );
    }
}

forge test --match-path test/foundry/MintAndAuction.t.sol --via-ir -vvvv

Tools Used

Manual Review and Foundry.

Recommended Mitigation Steps

Consider checking if collectionPhases[_collectionID].timePeriod is greater than 0 before executing certain logic, MinterContract.sol#L291-L294:

+		if (collectionPhases[_collectionID].timePeriod > 0) {
		// uint calculates if period has passed in order to allow minting
		uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[_collectionID].timePeriod;
		// users are able to mint after a day passes
		require(tDiff >= 1, "1 mint/period");
+		}

Assessed type

Math

[c4-pre-sort]

141345 marked the issue as duplicate of #1278

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-pre-sort]

141345 marked the issue as not a duplicate

[c4-pre-sort]

141345 marked the issue as duplicate of #1278

[c4-judge]

alex-ppg marked the issue as duplicate of #1980

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Overinflated severity