Switching to sales model 3 for a collection with pre-existing supply could brick the ' mint() ' function for that collection.

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

' Periodic sales' model cannot be used in a collection with Pre-existing supply because of how timeOfLastMint is calculated in mint() function.

This prevents the system from flexibly combining different sales models in a single collection as intended by the business model.

Proof of Concept

https://seize-io.gitbook.io/nextgen/nextgen-smart-contracts/features#minting-process

The docs of the NextGen Protocol (linked above) states the following,

* A collection can have an arbitrary number of phases (starting and ending times), 
each with their own allowlist.

* The sales models can be combined with phased allowlists to execute very complex drops.

Lets consider a common scenario followed in the NFT community, where Total Supply = 8500 collectionMintCost = 1 ETH 100 % of the total supply is allocate to allowlisted addresses, and any unminted NFTs go over to public sale phase.

Lets say 8000 NFTs are minted in allowlist phase.

The collection's admin wants to utilize ' Periodic sales' (sales model 3) for unminted tokens in a public sale phase.

In this case, the lastMintDate[col] would be assigned a value that is much higher due to gencore.viewCirSupply(col) in the formula

lastMintDate[col] = collectionPhases[col].allowlistStartTime + (collectionPhases[col].timePeriod * (gencore.viewCirSupply(col) - 1));

This means timeOfLastMint will be much greater than block.timestamp and tDiff will be 0.

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

This effectively reverts due to the following check,

require(tDiff>=1 && _numberOfTokens == 1, "1 mint/period");

preventing the NextGen system from flexibly combining different sales models in a single collection as intended by the business model.

Tools Used

Manual Analysis, Foundry.

Recommended Mitigation Steps

Remodel the way to calculate ' lastMintDate[col] '
by tracking the number of NFTs minted through sales model 3 using a separate counter without taking the collection's current ciculating supply into account.

Assessed type

Math

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-sponsor]

a2rocket (sponsor) confirmed

[alex-ppg]

The Warden has marked a direct discrepancy between the NextGen system's documentation (expected behavior) as well as implementation (actual behavior) that would result in the minting phases of the system being incompatible between them if multiple are performed in sequence.

To note, this incompatibility between phases cannot be rectified by a re-configuration of the collection as its circulating supply would remain high meaning that collections as described by the exhibit would be permanently "bricked" from periodic sales. As such, I deem the high severity as apt for this finding.

While the submission is valid and has been accepted by the sponsor, I strongly advise the Warden to increase the quality of their future submissions by making use of bullet lists, better splitting of content, and correct usage of backticks for in-line code segments.

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as selected for report

[c4-judge]

alex-ppg marked the issue as primary issue

[c4-judge]

alex-ppg marked issue #380 as primary and marked this issue as a duplicate of 380