Auction Winner Can Retrieve Bid Amount Back Along With NFT

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Vulnerability allows the highest bidder (the auction winner) to reclaim their bid amount while also receiving the NFT.

Proof of Concept

The vulnerability stems from the timing of checks in the claimAuction and cancelBid functions and reentrancy.

• The claimAuction function only proceeds if the auction has ended, as verified by require(block.timestamp >= minter.getAuctionEndTime(_tokenid))
• The cancelBid function can be invoked as long as the auction hasn't ended, checked via require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended")
• The use of safeTransferFrom to transfer the NFT leads to a reentrancy attack. If the highestBidder is a contract with malicious logic in onERC721Received, it will invoke cancelBid and pass all require statements, and get his ETH back.
• Transaction won't fail when trying to send ETH to owner in next line, since success of payable(owner()).call{value: highestBid}("") didn't check.

Tools Used

Manual code review

Recommended Mitigation Steps

• Synchronize the timing checks in claimAuction and cancelBid.
• Review the sequence of operations and state changes in the claimAuction function to prevent reentrancy. eg add auctionInfoData[_tokenid][index].status = false; before safeTransferFrom

Assessed type

Reentrancy

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as partial-50

[c4-judge]

alex-ppg marked the issue as full credit

[c4-judge]

alex-ppg marked the issue as partial-50