NextGen - merlin's results

Lines of code

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

Vulnerability details

Bug Description

The auctionDemo smart contract has three functions responsible for the NFT auction. All these functions, participateToAuction, claimAuction, and cancelBid, lack reentrancy checks and can be called when block.timestamp == minter.getAuctionEndTime(), meaning at the same time.

Proof-of-Concept

The participateToAuction, cancelBid, and claimAuction functions can be called in the last second of the auction:

block.timestamp <= minter.getAuctionEndTime(_tokenid)
block.timestamp >= minter.getAuctionEndTime(_tokenid)

Additionally, there are no reentrancy checks in the aforementioned functions, and low-level calls are not checked for success.

Let's consider the following scenario:

Grouping all the aforementioned issues, we have the following scenario: Preconditions: There are two different auctions for NFT with id 1, the highest bid being 3 ETH, and for NFT with id 2, the highest bid being 5 ETH.

1. A malicious user participates with a small bid (1 wei) in the auction for NFT id - 1.
2. Other users also participate in the auction, and the highest bid reaches 3 ETH.
3. In the last second of the auction, the malicious user rebids for the auction with NFT 1 with a bid of 4 ETH, executes claimAuction and cancelBid in the same transaction. Additionally, other users may also cancelBid in the same block, even after a refund in the claimAuction function.

Impact

The user can obtain the NFT and claim ETH for participating.

Tools Used

Manual

Recommended Mitigation Steps

Consider changing claimAuction as follows:

function claimAuction(
        uint256 _tokenid
    ) public WinnerOrAdminRequired(_tokenid, this.claimAuction.selector) {
        require(
-            block.timestamp >= minter.getAuctionEndTime(_tokenid) &&
+            block.timestamp > minter.getAuctionEndTime(_tokenid) &&
                auctionClaim[_tokenid] == false &&
                minter.getAuctionStatus(_tokenid) == true
        );

		///code
}

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Bug Description

When a user calls one of the following functions: mint, burnToMint, or burnOrSwapExternalToMint, the current price for the NFT is calculated, depending on the collection type. The issue arises in sale model 2, specifically when a user mints an NFT in the last second, resulting in the acquisition of the NFT at an incorrect amount.

All the above functions have a time check implemented as follows:

block.timestamp >= collectionPhases[col].publicStartTime &&
            block.timestamp <= collectionPhases[col].publicEndTime

The getPrice function for sale model 2 checks as follows:

block.timestamp >
            collectionPhases[_collectionId].allowlistStartTime &&
            block.timestamp < collectionPhases[_collectionId].publicEndTime

As a result, when purchasing an NFT in the last second before the publicEndTime using sale model 2, the user is charged an incorrect price, and the transaction fails.

Impact

If the collection has a sales option of 2, the user must buy the NFT at the wrong price.

Tools Used

Manual

Recommended Mitigation Steps

Consider changing getPrice as follows:

else if (
            collectionPhases[_collectionId].salesOption == 2 &&
-            block.timestamp > collectionPhases[_collectionId].allowlistStartTime &&
-            block.timestamp < collectionPhases[_collectionId].publicEndTime

+            block.timestamp >= collectionPhases[_collectionId].allowlistStartTime &&
+            block.timestamp <= collectionPhases[_collectionId].publicEndTime
        )

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Bug Description

To participate in the auction, one needs to invoke the participateToAuction function and have the highest bid. To claim the NFT, the claimAuction function should be called. These two functions have incorrect auction end time checks, introducing a vulnerability to potential fund losses.

Proof-of-Concept

The issue lies in the auction end time verification:

/// participateToAuction
require( block.timestamp <= minter.getAuctionEndTime(_tokenid));

/// claimAuction
require(block.timestamp >= minter.getAuctionEndTime(_tokenid));

So, it is possible to participate and claim the NFT simultaneously. Let's consider the following scenario: Preconditions: The last highest bid is 8 ETH.

1. Bob participates in the auction (participateToAuction) at the last moment (block.timestamp == auctionEndTime) with a bid of 10 ETH.
2. Alice frontruns Bob's transaction, participates in the auction (participateToAuction) with a bid of 9 ETH, and simultaneously claims the NFT (claimAuction), sends ETH to the owner and refunds other users.
3. Bob's transaction executes in the same block and doesn't fail. After this, Bob won't be able to retrieve his funds through the cancelBid function.

Impact

The ETH used to participate in the auction will be lost.

Tools Used

Manual

Recommended Mitigation Steps

Consider changing claimAuction as follows:

function claimAuction(
        uint256 _tokenid
    ) public WinnerOrAdminRequired(_tokenid, this.claimAuction.selector) {
        require(
-            block.timestamp >= minter.getAuctionEndTime(_tokenid) &&
+            block.timestamp > minter.getAuctionEndTime(_tokenid) &&
                auctionClaim[_tokenid] == false &&
                minter.getAuctionStatus(_tokenid) == true
        );

		///code
}

Assessed type

Invalid Validation