Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing time period check will result in revert during minting #453

Closed
c4-submissions opened this issue Nov 7, 2023 · 8 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-1980 edited-by-warden unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Nov 7, 2023

Lines of code

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

Vulnerability details

In the mint(), if the nft is to be minted at certain intervals then the time period interval is set at collectionPhases[col].timePeriod.
But there are possible chances that the mint function will revert because there is no check to ensure that the time period isn't set to 0 during sales option 3.

Proof of Concept

File: MinterContract.sol

     // control mechanism for sale option 3
        if (collectionPhases[col].salesOption == 3) {
            uint timeOfLastMint;
            if (lastMintDate[col] == 0) {
                // for public sale set the allowlist the same time as publicsale
                timeOfLastMint = collectionPhases[col].allowlistStartTime - collectionPhases[col].timePeriod;
            } else {
                timeOfLastMint =  lastMintDate[col];
            }
            // uint calculates if period has passed in order to allow minting
249:        uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod;  /// @note - will revert if timePeriod is 0
            // users are able to mint after a day passes
            require(tDiff>=1 && _numberOfTokens == 1, "1 mint/period");
            lastMintDate[col] = collectionPhases[col].allowlistStartTime + (collectionPhases[col].timePeriod * (gencore.viewCirSupply(col) - 1));
        }

Take the following scenario:

  • Collection has a rate & time period greater than 0 for variable change in price per mint & time gap.
  • collectionPhases[col].salesOption == 3 in this case.
  • After sometime it is decided to do a 1 hour special mint where the price will be fixed & no time gap between mints.
  • This means both rate & time period will be set to 0 which is done by the admin.
  • Now the mint() is executed & it enters if (collectionPhases[col].salesOption == 3) block.
  • tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod on line 249 will revert because of division by 0 because the time period has been set to 0.
  • Thus during this time all the mints will revert.

Impact

Due to missing check, it is possible that minting will revert in case the time period is set to 0 during sales option 3.

Tools Used

Manual

Recommended Mitigation Steps

Add a check in the if block where the sales option is checked for time period to be greater than 0.

File: MinterContract.sol

- 240:   if (collectionPhases[col].salesOption == 3) {
+ 240:   if (collectionPhases[col].salesOption == 3 && collectionPhases[col].timePeriod > 0) {

Assessed type

Invalid Validation

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 7, 2023
c4-submissions added a commit that referenced this issue Nov 7, 2023
@code4rena-admin code4rena-admin changed the title Users can mint tokens consecutively without waiting for the time period to pass Missing time period check will result in revert during minting Nov 7, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #478

@c4-pre-sort
Copy link

141345 marked the issue as not a duplicate

@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1278

@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #962

@c4-pre-sort
Copy link

141345 marked the issue as not a duplicate

@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1278

@c4-judge
Copy link

c4-judge commented Dec 6, 2023

alex-ppg marked the issue as duplicate of #1980

@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as unsatisfactory:
Overinflated severity

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-1980 edited-by-warden unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants