NextGen - 0xarno's results

Lines of code

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

Vulnerability details

Impact

Malicious bidders can steal ETH twice the amount they bid by exploiting cross-function reentrancy.

Proof of Concept

Consider the following scenario with participants Alice, Bob, and Bob's second account (let's call it Ron) in the auction:

1)Alice, Ron, and Bob bid 1 ETH, 2 ETH, and 3 ETH, respectively.

2)Bob, as the highest bidder, calls claimAuction when block.timestamp == minter.getAuctionEndTime(_tokenId).

3)The claimAuction function also refunds the amount to the bidders who lost the auction. Once the winner claims the auction or the function admin calls it.

4)During the loop iteration, the amount is refunded to Ron. Ron can then re-enter the cancelBid function to cancel the bid as soon as it receives the funds, effectively receiving the funds twice, Ron receives 4 ETH.

5)Upon the loop's third iteration, the NFT is sent to the winner, and the function execution finishes.

Link to relevant code - Line 116

Link to relevant code - Line 124

Tools Used

vscode

Recommended Mitigation Steps

Ensure that the claimAuction function is called only after the auction has ended.

- require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
+ require(block.timestamp > minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

The winner of the auction can render the claimAuction function useless for a particular ID by bidding through a smart contract and not adding the onERC721Received callback hook. This callback is called by the NFT contract to ensure that the smart contract is capable of receiving and transferring the NFT. The malicious bidder will lose funds for a particular auction, and other bidders will not receive refunds because the function will revert each time.

Proof of Concept

Consider the following scenario with participants Alice and Bob in the auction:

A) Alice bids 1 ETH for tokenId = 2. B) Other bidders participate in the auction for the same tokenId. C) Bob places the highest bid (for tokenId = 2) and becomes the winner of the auction after it ends. However, he bid using a smart contract that did not implement the onERC721Received callback hook, either intentionally or unintentionally. D) If the winner or admin calls the claimAuction function for the same tokenId, IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenId); will revert. This is because the callback checks whether the recipient contract is capable of receiving ERC721, but in this case, it is not, resulting in a revert.

Link to relevant code - Line 112

Tools Used

VSCode

Recommended Mitigation Steps

Use transferFrom instead of safeTransferFrom.

Assessed type

DoS

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#L58

Vulnerability details

Impact

In the AuctionDemo.sol contract, a bidder can participate in the auction using the participateToAuction function. However, there is an edge case where bidders can lose money if they bid when block.timestamp == AuctionEndTime and their transaction is below the winner's transaction in the block. In this scenario, the winner may intentionally or unintentionally pay a high gas fee to include their transaction at the top of the block, causing other transactions to be processed after theirs. Participants cannot cancel their bids because the time would have passed, and there is no recovery function to retrieve ETH from the contract.

Proof of Concept

Let's consider the following scenario with participants Alice, Ron, and Bob in the auction:

1. Alice bids 1 ETH using participateToAuction(tokenId = 1).
2. Bob waits until the last moment to bid, checking for other bidders.
3. Bob bids 1.1 ETH for tokenId 1 in the block where time = AuctionEndTime.
4. Alice sees this transaction in the mempool and calls the claimAuction function with tokenId = 1, using a higher gas price than Bob. Alternatively, Alice can use MEV to include her transaction in the specific block.
5. Although Alice has already claimed the auction, Bob's bidding transaction will be processed after Alice's transaction. As a result, he will lose his 1.1 ETH because the time period has already passed, and there are no other bidders for Alice.

Tools Used

vscode

Recommended Mitigation Steps

start claiming of auction only after bidding is finished

Assessed type

Other