AuctionDemo#claimAuction - A malicious user could steal the NFT by being the winner and canceling their bid on the last block timestamp #1896
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate-1323
partial-50
Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Lines of code
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L105
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L125
Vulnerability details
Impact
The vulnerability arises due to a timing issue where
block.timestamp
is used as a condition to gate both theclaimAuction
andcancelBid
functions. Specifically, whenblock.timestamp
is equal tominter.getAuctionEndTime(_tokenId)
, a user can invoke both functions in the same transaction block. This is because the conditional check uses<=
instead of<
. Consequently, a malicious actor could claim the auctioned token withclaimAuction
and then invalidate their bid by callingcancelBid
, retrieving their bid amount and effectively receiving the token without payment.Proof of Concept
The attack can be executed as follows:
block.timestamp
is exactly equal tominter.getAuctionEndTime(_tokenId)
, executeclaimAuction
.cancelBid
. Due to the conditionblock.timestamp <= minter.getAuctionEndTime(_tokenId)
, this will succeed.Tools Used
Manual Review.
Recommended Mitigation Steps
cancelBid
cannot be called in the same block asclaimAuction
. This can be achieved by using a strict inequality (<
) incancelBid
.auctionClaimed
totrue
whenclaimAuction
is successfully called. This variable should then be checked in thecancelBid
function to prevent bid cancellation after a claim (require(auctionClaim[_tokenid] = false
)Assessed type
Invalid Validation
The text was updated successfully, but these errors were encountered: