NextGen - Krace's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L105 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L125 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L135

Vulnerability details

Impact

Bidders with the highest bid have the potential to double their bidding funds. This can be achieved by consecutively calling the claimAuction and cancelAllBids functions, provided that both functions are executed when block.timestamp is equal to getAuctionEndTime(_tokenid).

Proof of Concept

claimAuction allows the highestBidder to cliam the corresponding _tokndid when the auction is over.

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    //@audit timestamp can be equal to `EndTime`
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
        auctionClaim[_tokenid] = true;
        uint256 highestBid = returnHighestBid(_tokenid);
        address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
        address highestBidder = returnHighestBidder(_tokenid);
        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            } else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

Bidders can cancel bids before the auction ends using the function cancelBid or cancelAllBids. These two functions will refund the funds to bidders and mark the status of auctionInfoData as false.

    function cancelBid(uint256 _tokenid, uint256 index) public {
    //@audit timestamp can be equal to EndTime
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
        auctionInfoData[_tokenid][index].status = false;
        (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
        emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
    }

    function cancelAllBids(uint256 _tokenid) public {
    //@audit timestamp can also be equal to EndTime
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
                auctionInfoData[_tokenid][i].status = false;
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
            } else {}
        }
    }

However, the timestamp of claimAuction and cancel* overlap when block.timestamp == minter.getAuctionEndTime(_tokenid). Due to that claimAuction will only return the funds to bidders but not set the status of auctionInfoData, bidders could call cancelAllBids to get the funds again.

Example

Here is an example:

Alice is the highest bidder and she bids with [5, 10, 15, 20]. When block.timestamp == minter.getAuctionEndTime(_tokenid), Alice calls cliamAuction to get the _tokenid and ETH=5+10+15=30. After that, Alice calls cancelAllBids at the same block.timestamp, and she will get 5+10+15+20=50 ETH.

In addition to the cancelAllBids, the cancelBid can also achieve the same effect.

Tools Used

Manual Review

Recommended Mitigation Steps

You can change the lock.timestamp >= minter.getAuctionEndTime(_tokenid) to lock.timestamp > minter.getAuctionEndTime(_tokenid) in function claimAuction to prevent cancellations after claim.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The MinterContract.sol fails to guarantee the minting of only one token in each period for Periodic Sale collections. This discrepancy occurs because functions like airDropTokens, burnOrSwapExternalToMint, and burnToMint have the capacity to mint Periodic Sale tokens at any time. Consequently, the mint function need to wait for additional time to mint Periodic Sale tokens.

Proof of Concept

As outlined in the docs, Periodic Sale(Option 3) collections are designed to permit the minting of only one token during each time period.

Mints are limited to 1 token during each time period (e.g. minute, hour, day). The minting cost can either stable or have a bonding curve (increase) over time. This allows for long-lived or experimental mints.

In the NextGenMinterContract, the mint function is responsible for verifying the salesOption and updating the lastMintDate of a collection. The value of lastMintDate is computed based on the number of tokens already minted in this collection, which is obtained through gencore.viewCirSupply(col). If there are 10 tokens minted, then the lastMintDate should be allowlistStartTime+period*9. This logic functions correctly when mint is the sole function capable of minting Periodic Sale tokens.

    function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
[....]
        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));
        }
    }

However, other functions, including airDropTokens, burnOrSwapExternalToMint, and burnToMint, can also mint Periodic Sale tokens. For example, the burnToMint function has the ability to directly mint Periodic Sale tokens without any restrictions. This increases the value of gencore.viewCirSupply(col), leading to a larger value for lastMintDate. Consequently, the mint function must wait a longer period to mint the next Periodic Sale token.

    function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o) public payable {
        require(burnToMintCollections[_burnCollectionID][_mintCollectionID] == true, "Initialize burn");
        require(block.timestamp >= collectionPhases[_mintCollectionID].publicStartTime && block.timestamp<=collectionPhases[_mintCollectionID].publicEndTime,"No minting");
        require ((_tokenId >= gencore.viewTokensIndexMin(_burnCollectionID)) && (_tokenId <= gencore.viewTokensIndexMax(_burnCollectionID)), "col/token id error");
        // minting new token
        uint256 collectionTokenMintIndex;
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_mintCollectionID), "No supply");
        require(msg.value >= getPrice(_mintCollectionID), "Wrong ETH");
        uint256 mintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
        // burn and mint token
        address burner = msg.sender;
        gencore.burnToMint(mintIndex, _burnCollectionID, _tokenId, _mintCollectionID, _saltfun_o, burner);
        collectionTotalAmount[_mintCollectionID] = collectionTotalAmount[_mintCollectionID] + msg.value;
    }

Case

Let's consider a scenario where we have a collection1 with a Periodic Sale type, starting at 1:00 with a period of 1 hour:

1. At 1:00, user1 mints a token in collection1 using the mint function. The value of gencore.viewCirSupply(col) is 1, and the lastMintDate is set to 1:00 + (1 - 1) = 1:00.

2. Prior to 2:00, ten users employ burnToMint to mint tokens in collection1. Consequently, gencore.viewCirSupply(col) reaches 11.

3. After 2:00, user1 again invokes the mint function for collection1. As a result, gencore.viewCirSupply(col) increases to 12, and the lastMintDate is recalibrated to 1:00 + (12 - 1) = 12:00.

In this scenario, the mint function will have to wait an additional 10 hours to mint the next collection1 token. However, burnToMint can still mint new tokens in collection1.

Tools Used

Manual Review

Recommended Mitigation Steps

To resolve this issue, it is essential to implement a restriction in the contract NextGenMinterContract that ensures only one token can be minted in every period, as specified for Periodic Sale collections.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The mintAndAuction function may encounter extended wait times to mint and auction a new token due to an incorrect calculation of lastMintDate.

Proof of Concept

As documented in the docs, only one token is permitted to be minted and auctioned during each time period.

One token can be minted and auctioned at each time period, thus is essential to call the setCollectionCosts(..) function and set a timeperiod.

The lastMintDate variable is utilized to monitor the passing of periods (one token per period), and it is calculated based on the total number of minted tokens, determined by gencore.viewCirSupply(col). However, it's crucial to acknowledge that other methods, such as mint and airDrop, can also mint tokens. Thus, if tokens are minted through these alternative methods between two mintAndAuction calls, the lastMintDate will be updated to a timestamp significantly exceeding the current timestamp.

    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;
        // 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;
    }

Let's illustrate this with a scenario. Consider collection1 with a start time at 1:00 and a period of 1 hour:

1. At 1:00, user1 initiates the minting and auctioning of a token in collection1 via mintAndAuction. At this point, gencore.viewCirSupply(col) is 1, and lastMintDate is set to 1:00 + (1 - 1) = 1:00.

2. Prior to 2:00, ten users mint tokens in collection1. Consequently, gencore.viewCirSupply(col) reaches 11.

3. After 2:00, user1 invokes mintAndAuction again for collection1. This results in gencore.viewCirSupply(col) being updated to 12, and the lastMintDate is recalibrated to 1:00 + (12 - 1) = 12:00.

As a result, mintAndAuction is forced to wait an additional 10 hours to mint and auction the next token in collection1.

Tools Used

Mannual

Recommended Mitigation Steps

To resolve this issue, it is advisable to introduce a new variable, lastMintAndAuctionDate, dedicated to recording the time when mintAndAuction is called. This date should be calculated using a separate counter, ensuring precise time period calculations between various minting and auctioning events.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

Minting at publicEndTime may result in higher minting costs for users than they expected.

Proof of Concept

The Attack idea of Sales Models says that:

Consider ways in which the minting cost price will differ from the actual value based on the parameters set on the setCollectionCosts() function.

When calculating the price of Descending Sale, the getPrice function checks if the block.timestamp is less than the publicEndTime. Therefore, if the block.timestamp is equal to the collectionPhases[_collectionId].publicEndTime, getPrice will directly return a fixed price, which is much higher than the expected minting cost price.

    function getPrice(uint256 _collectionId) public view returns (uint256) {
        uint tDiff;
        if (collectionPhases[_collectionId].salesOption == 3) {
            // increase minting price by mintcost / collectionPhases[_collectionId].rate every mint (1mint/period)
            // to get the price rate needs to be set
            if (collectionPhases[_collectionId].rate > 0) {
                return collectionPhases[_collectionId].collectionMintCost + ((collectionPhases[_collectionId].collectionMintCost / collectionPhases[_collectionId].rate) * gencore.viewCirSupply(_collectionId));
            } else {
                return collectionPhases[_collectionId].collectionMintCost;
            }
        } 
        //@audit if timestamp == collectionPhases[_collectionId].publicEndTime, this condition is not satisfied and getPrice will return a fixed price.
        else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
            // decreases exponentially every time period
            // collectionPhases[_collectionId].timePeriod sets the time period for decreasing the mintcost
            // if just public mint set the publicStartTime = allowlistStartTime
            // if rate = 0 exponetialy decrease
            // if rate is set the linear decrase each period per rate
            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));
            } else {
                if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
                    price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
                } else {
                    price = collectionPhases[_collectionId].collectionEndMintCost;
                }
            }
            if (price - decreaserate > collectionPhases[_collectionId].collectionEndMintCost) {
                return price - decreaserate; 
            } else {
                return collectionPhases[_collectionId].collectionEndMintCost;
            }
        } else {
        //@audit return the fixed price when `block.timestamp` is equal to `collectionPhases[_collectionId].publicEndTime`
            // fixed price
            return collectionPhases[_collectionId].collectionMintCost;
        }
    }

Here is an Exponential Descending Sale Example from the document, the price decreases gradually each second until it reaches the resting price 30mins (3 periods) after the start of the sale and stays there until the end of the sale.

As time goes on, the price of the Descending Sale decreases. Therefore, at the publicEndTime, the price should be the lowest. However, getPrice returns the original fixed price, which is significantly higher than user expectations.

This can be easily achieved because the checks in the mint, butnToMint and burnOrSwapExternalToMint functions all require that block.timestamp <= collectionPhases[col].publicEndTime.

[...]
        //@audit the check in `mint` shows that block.timestamp can be equal to `publicEndTime`
        } else if (block.timestamp >= collectionPhases[col].publicStartTime && block.timestamp <= collectionPhases[col].publicEndTime) {
            phase = 2;
            require(_numberOfTokens <= gencore.viewMaxAllowance(col), "Change no of tokens");
            require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");
            mintingAddress = msg.sender;
            tokData = '"public"';
        } 
[...]

Tools Used

Manual Review

Recommended Mitigation Steps

It is advisable to change the block.timestamp < collectionPhases[_collectionId].publicEndTime in getPrice to block.timestamp <= collectionPhases[_collectionId].publicEndTime.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

This error resulted in the transfer of the highest bid funds to the wrong recipient, leading to a loss of funds for the token owner.

Proof of Concept

As outlined in the README, the owner of the token should receive the funds.

• The highest bidder will receive the token after an auction finishes, the owner of the token will receive the funds and all other participants will get refunded.

However, during the execution of the claimAuction function, the funds from the highest bid are erroneously transferred to the owner of the auctionDemo contract instead of the owner of the _tokenid.

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
        auctionClaim[_tokenid] = true;
        uint256 highestBid = returnHighestBid(_tokenid);
        address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
        address highestBidder = returnHighestBidder(_tokenid);
        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
                //@audit funds should be transferred to the ownerOfToken
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            } else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

Tools Used

Mannual Review

Recommended Mitigation Steps

To address this issue, it is suggested that the funds be transferred to the owner of the token. Here is the modified code snippet:

diff --git a/smart-contracts/AuctionDemo.sol b/smart-contracts/AuctionDemo.sol
index 95533fb..41b52db 100644
--- a/smart-contracts/AuctionDemo.sol
+++ b/smart-contracts/AuctionDemo.sol
@@ -110,8 +110,8 @@ contract auctionDemo is Ownable {
         for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
             if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                 IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
-                (bool success, ) = payable(owner()).call{value: highestBid}("");
-                emit ClaimAuction(owner(), _tokenid, success, highestBid);
+                (bool success, ) = payable(ownerOfToken).call{value: highestBid}("");
+                emit ClaimAuction(ownerOfToken, _tokenid, success, highestBid);
             } else if (auctionInfoData[_tokenid][i].status == true) {
                 (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                 emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);

Assessed type

Context