The winner of the auction could DOS the claimAuction function resulting in loss of fund of anyone who participated

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Note: 1. this requires the attacker to lose his highest bid and lose out on the NFT 2. unlike the bot report this is not due to the loop DOS.

When the auction is over, claimAuction() loops over the bids and either 1. sends the NFT to the highest bidder and 2. returns the bids to the participants.
However, if the highest bidder reverts the transfer on purpose, no one will be able to get their bids value back. This is due to the fact that when calling the function, it will loop over all bids and eventually end up in the transfer call.

Proof of Concept

1. Attacker sends highest bid via a contract
2. Attacker sets _checkOnERC721Received to revert (or does not implement it at all)
3. When the admin calls claimAuction(), it will always revert

Tools Used

Manual review

Recommended Mitigation Steps

There are two ways to handle this.

1. Implement a failsafe for the functions when they revert. This could be done by continuing the function but saving the revert values for a later moment, for example with a try catch.
   If the try attempt fails, save the values (bid amount and bidder) into a mapping and provide an additional function to retrieve those funds at a later time.
2. Redesign the claimAuction() without the modifier and where the transfer only occurs if msg.sender == auctionInfoData[_tokenid][i].bidder

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #486

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1759

[c4-judge]

alex-ppg marked the issue as partial-50