code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

An airdrop of a collection with the sell option 3 leads to an incorrect calculation of the number of passed periods during the sales phase #1821

Closed c4-submissions closed 11 months ago

c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The mint function contains a control mechanism for sell option 3, ensuring that only one token can be minted per period.

function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
    ...
    // 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
        uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod;
        // 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));
    }
}

If lastMintDate[col] == 0 (misinterpreted as indicating no tokens have been minted), the timeOfLastMint is calculated as collectionPhases[col].allowlistStartTime - collectionPhases[col].timePeriod, overlooking viewCirSupply - the number of minted tokens.

At the same time, the value of viewCirSupply may not be zero if there was an airdrop before. This is because the airDropTokens function mints the token but doesn't set lastMintDate.

function airDropTokens(address[] memory _recipients, string[] memory _tokenData, uint256[] memory _saltfun_o, uint256 _collectionID, uint256[] memory _numberOfTokens) public FunctionAdminRequired(this.airDropTokens.selector) {
    require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
    uint256 collectionTokenMintIndex;
    for (uint256 y=0; y< _recipients.length; y++) {
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID) + _numberOfTokens[y] - 1;
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");
        for(uint256 i = 0; i < _numberOfTokens[y]; i++) {
            uint256 mintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
            gencore.airDropTokens(mintIndex, _recipients[y], _tokenData[y], _saltfun_o[y], _collectionID);
        }
    }
}

The subsequent assignment of the lastMintDate takes into account the viewCirSupply:

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

Thus, if there was an airdrop, then one token can be minted after the sales phase started anyway, but the next mint is only possible after timePeriod multiplied by the number of airdropped tokens.

Depending on the design, there may be different expectations regarding how it should work correctly, but one of the considered cases is not valid.

Proof of Concept

Let's look at an example:

First mint call after the sales phase starts:

Second mint call:

Tools Used

Manual review

Recommended Mitigation Steps

Depending on the intended design:

  1. During the first calculation of timeOfLastMint, consider the current viewCirSupply, preventing unintended mints during the sale phase.
  2. Alternatively, subtract the number of airdropped tokens from the calculation of lastMintDate. This prevents airdrops from affecting the next allowed minting time.

Assessed type

Other

c4-pre-sort commented 12 months ago

141345 marked the issue as duplicate of #246

c4-judge commented 11 months ago

alex-ppg marked the issue as not a duplicate

c4-judge commented 11 months ago

alex-ppg marked the issue as duplicate of #2012

c4-judge commented 11 months ago

alex-ppg changed the severity to 3 (High Risk)

c4-judge commented 11 months ago

alex-ppg marked the issue as partial-50