Submitted by The_Kakers, also found by Madalad, immeas, DeFiHackLabs, Neon2835, xeros, Bauchibred, fibonacci, 00decree, SpicyMeatball, ChrisTina, Haipls, _eperezok, bdmcbri, alexxander, 0xarno, tnquanghuy0512, Ocean_Sky, spark, Talfao, r0ck3tz, Arabadzhiev, Tricko, 00xSEV, Jiamin, nuthan2x, lsaudit, cu5t0mpeo, 0x_6a70, bronze_pickaxe, circlelooper, rotcivegaf, Nyx, KupiaSec, Delvir0, BugsFinder0x, crunch, ke1caM, twcctop, Juntao, 0xJuda, amaechieth, 0xpiken, funkornaut, 0x180db, Taylor_Webb, BugzyVonBuggernaut, HChang26, ZdravkoHr, Timenov, dimulski, and 0x3b
An auction winner can decide to conditionally revert the claimAuction() execution.
This leads to the following impacts:
This can lead to permanent loss of funds if the adversary decides to. As a drawback, they cant get the NFT.
But nevertheless, they can use their control over the function to ask for a ransom, as the sum of other bids, plus the protocol earnings can be higher than the maximum bid itself and the damaged public image of the protocol for losing users funds.
It also breaks one of the main invariants of the protocol:
Evaluated as Medium, as despite of the possibility of permanent assets lost, and breaking a main invariant, the adversary will have to take a loss as well; or persuade the participants via a ransom.
claimAuction() transfers the NFT using safeTransferFrom():
safeTransferFrom() calls back the receiver if it is a contract.
An adversary that won the auction can conditionally revert the execution of the transaction, upon the onERC721Received() callback.
This will revert the whole transaction, making it impossible to transfer any funds.
Bidders cant cancel their bids either via cancelBid() or cancelAllBids(), due to they only allow it to do so if the auction was not ended: AuctionDemo.sol#L125
The protocol, nor the previous token owner have a way to claim the earnings from the auction.
Move the logic to transfer the NFT to its own separated function.
DoS
0xsomeone (judge) commented:
btk (warden) commented:
0xsomeone (judge) commented:
a2rocket (NextGen) confirmed
Note: For full discussion, see here.
