mintAndAuction() can be broken easily, and the auction flow can be permanently disabled for a collection.

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Some collection id can have a parallel auction as well as regular phased minting. And mintAndAuction() enforces a periodic minting check to limit one token that can be minted for auction at a given time period.

This issue is that this check is enforced with a flawed logic that allows mintAndAuction() to be broken for a collection.

Proof of Concept

In MinterContract.sol - mintAndAuction(), whenever a collection allows minting, the function admin can mint one token per period. This is ensured through tracking lastMintDate[_collectionId] to calculated the time passed since the last mint tDiff = (block.timestamp - timeOfLastMint) /collectionPhases[_collectionID].timePeriod. And the first timeOfLastMint value is based on the collection allowListStarTime. This means that mintAndAuction() start time is the same as the general mint().

However, the way lastMintDate[_collectionId] is updated is flawed. Currently, lastMintDate[_collectionId] is not updated strictly based on block.timestamp, but rather updated based on the current circulation supply of a token. This means that if the collection circulation supply is increased, the lastMintDate[_collectionId] will be increased as well.

And the collection circulation supply can be increased through many other methods such as airDropTokens(), mint() or burnToMint(), etc. For example, if users are minting new tokens for the collection through the publicly accessible mint() function, they can mint many tokens depending on the sales model and mint model.

This means that when a user can mint directly through mint() to cause the token circulation supply to increase such that in mintAndAuction() - lastMintDate[_collectionId]will be set as an unrealistic future timestamp, this will cause(block.timestamp - timeOfLastMint)` to underflow, effectively disable all auction for the collection.

// smart-contracts/MinterContract.sol
    function mintAndAuction(
        address _recipient,
        string memory _tokenData,
        uint256 _saltfun_o,
        uint256 _collectionID,
        uint _auctionEndTime
    ) public FunctionAdminRequired(this.mintAndAuction.selector) {
...
       uint timeOfLastMint;
        // check 1 per period
        if (lastMintDate[_collectionID] == 0) {
            // for public sale set the allowlist the same time as publicsale
            timeOfLastMint =
                collectionPhases[_collectionID].allowlistStartTime -
                collectionPhases[_collectionID].timePeriod;
        } else {
            timeOfLastMint = lastMintDate[_collectionID];
        }
        // 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");
// @audit lastMintDate[] is calculated based on gencore.viewCirSupply(_collectionID) as supposed to `block.timestamp`
 |>     lastMintDate[_collectionID] =
            collectionPhases[_collectionID].allowlistStartTime +
            (collectionPhases[_collectionID].timePeriod *
                (gencore.viewCirSupply(_collectionID) - 1));
        mintToAuctionData[mintIndex] = _auctionEndTime;
        mintToAuctionStatus[mintIndex] = true;
    }

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

// smart-contracts/NextGenCore.sol
    function mint(
        uint256 mintIndex,
        address _mintingAddress,
        address _mintTo,
        string memory _tokenData,
        uint256 _saltfun_o,
        uint256 _collectionID,
        uint256 phase
    ) external {
...
//@audit-info Note: mint will always increase .collectionCirculationSupply
        collectionAdditionalData[_collectionID].collectionCirculationSupply =
            collectionAdditionalData[_collectionID]
                .collectionCirculationSupply +
            1;
...

(https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L191)

As seen above, when any of other minting methods such as mint() from MinterContract.sol invokes mint() on NextGenCore.sol, collectionCirculationSupply will increase, which means back in mintAndAuction(), lastTimeDate[_collectionId] will directly be updated to a future timestamp when anyone minted more than one token within a given period. And this will cause the future mintAndAuction() to revert due to block.timestamp - timeOfLastMint underflow.

This vulnerability can either be exploited by a user maliciously minting to cause the auction flow to revert, or by the normal operation of public minting of excessive amounts of tokens.

Tools Used

Manual review

Recommended Mitigation Steps

Consider changing how lastMintDate[_collectionId] is calculated, calculating the timestamp based on block.timestamp such that the value will always reflect a historic timestamp.

Assessed type

Error

[c4-pre-sort]

141345 marked the issue as duplicate of #688

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1613

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid