NextGen - Zach_166's results

Lines of code

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

Vulnerability details

Impact

The requirements for executing claimAuction() and cancelBid()/cancelAllBids() are currently set as block.timestamp <= minter.getAuctionEndTime(_tokenid) and block.timestamp >= minter.getAuctionEndTime(_tokenid), respectively. However, within a single block, the block.timestamp remains the same, which could be set by the miner. As a result, if a miner set the block.timestamp = minter.getAuctionEndTime(_tokenid), both conditions could be satisfied simultaneously. In such a case, the winner can not only claim the auctioned NFT but also withdraw the invested ETH by calling the cancelBid()/cancelAllBids() in the same block. Since claimAuction() does not verify whether all bidders have received a refund (i.e., whether transferfrom() was successfully executed), the winner can obtain the auctioned NFT at no cost, and some participants in the auction may incur losses (not winning the NFT and unable to retrieve the bidding ETH).

Proof of Concept

Here are the steps of the attack:

1. When a Ethereum miner is currently minting a block near the end time of the auction, and then set the block.timestamp as the same to the end time of the auction.
2. The miner calls participateToAuction() to place the highest bid and win the auction. Immediately afterward, the miner calls claimAuction() and cancelBid().

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended that the project modify the conditions for claimAuction() and cancelBid()/cancelAllBids() to block.timestamp < minter.getAuctionEndTime(_tokenid) and block.timestamp > minter.getAuctionEndTime(_tokenid), respectively (by removing the equal sign).

Assessed type

Timing