minting of new tokens prevented with timePeriod of zero

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L249
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#L546
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L551

Vulnerability details

Impact

When the timePeriod in collectionPhases is set to zero (as it is in nextGen.test.js and as it is described in the documentation), several functions
can revert due to divide by zero errors. These functions include, mintToAuction:

    function mintAndAuction(address _recipient, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint _auctionEndTime) public FunctionAdminRequired(this.mintAndAuction.selector) {
        require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
        uint256 collectionTokenMintIndex;
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");
        uint256 mintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
        gencore.airDropTokens(mintIndex, _recipient, _tokenData, _saltfun_o, _collectionID);
        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;             //<<<<< Will result in divide by zero!
        // users are able to mint after a day passes
        require(tDiff>=1, "1 mint/period");
        lastMintDate[_collectionID] = collectionPhases[_collectionID].allowlistStartTime + (collectionPhases[_collectionID].timePeriod * (gencore.viewCirSupply(_collectionID) - 1));
        mintToAuctionData[mintIndex] = _auctionEndTime;
        mintToAuctionStatus[mintIndex] = true;
    }

This results in not being able to mint tokens for an auction.

Additional functions fail to function correctly depending on the collection phase and rate.

getPrice:

            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));

and then in mint:

            // uint calculates if period has passed in order to allow minting
            uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod;

Proof of Concept

Testing with the following parameters from nextGen.test.js result in a revert when trying to mint a token for an auction:

  context("Set Collection Costs and Phases", () => {
    it("#setCollectionCost1", async function () {
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        0, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )
    })

Tools Used

VScode. Foundry

Recommended Mitigation Steps

1. In setCollectionCosts, if _salesOption is not equal to 1, then enforce a non-zero timePeriod. This fixes getPrice() and mint.
2. In mintAndAuction, if salesOption!=3, then don't calculate tDIff. This is the only salesOption where timePeriod can be zero, but then tDiff isn't applicable.

The below diff fixes the issue:

diff --git a/smart-contracts/MinterContract.sol b/smart-contracts/MinterContract.sol
index df50841..c4219f9 100644
--- a/smart-contracts/MinterContract.sol
+++ b/smart-contracts/MinterContract.sol
@@ -156,6 +156,7 @@ contract NextGenMinterContract is Ownable {

     function setCollectionCosts(uint256 _collectionID, uint256 _collectionMintCost, uint256 _collectionEndMintCost, uint256 _rate, uint256 _timePeriod, uint8 _salesOption, address _delAddress) public CollectionAdminRequired(_collectionID, this.setCollectionCosts.selector) {
         require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
+        if (_salesOption != 1 && _timePeriod == 0) { revert("Non-zero time period is required"); }
         collectionPhases[_collectionID].collectionMintCost = _collectionMintCost;
         collectionPhases[_collectionID].collectionEndMintCost = _collectionEndMintCost;
         collectionPhases[_collectionID].rate = _rate;
@@ -288,10 +289,12 @@ contract NextGenMinterContract is Ownable {
         } 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");
+        if (collectionPhases[_collectionID].salesOption == 3) {
+            // 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");
+        }
         lastMintDate[_collectionID] = collectionPhases[_collectionID].allowlistStartTime + (collectionPhases[_collectionID].timePeriod * (gencore.viewCirSupply(_collectionID) - 1));
         mintToAuctionData[mintIndex] = _auctionEndTime;
         mintToAuctionStatus[mintIndex] = true;

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