Claiming auctions can be DDoS because of the push payments, resulting in stuck funds and NFT not being distributed

[c4-submissions]

Lines of code

Function for claiming an auction: https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L104-L120
Line that will be able to exploit to create de DoS: https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L116

Vulnerability details

Bug Description

• Users claim auctions using the claimAuction function, which facilitates the refund of corresponding bids to the unsuccessful bidders. This mechanism employs push payments with ETH.

                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");

Making push payments like this it's really dangerous, because if any of the bidders to be refunded make the transaction revert, claimAuction won't be callable.

Impact

The vulnerability causes all calls to claimAuction to revert, resulting in a Denial of Service scenario and hindering the proper distribution of NFTs and funds. All the funds used for bidding in that auction will be stuck in the contract forever.

Proof of Concept

Attack Path

1. An attacker develops a smart contract capable of bidding in auctions via the participateToAuction function. Additionally, the contract is designed to revert upon receiving any ETH.
2. The attacker employs the contract to place a bid of 1 wei as an auction commences.
3. The auction progresses as usual, with multiple bids leading to a winner.
4. Bob, the winner of the auction, attempts to claim his NFT using claimAuction, resulting in transaction reversion consistently.
   4.1. This failure occurs because the contract aims to refund the unsuccessful bidders' ETH bids. When attempting to refund the attacker's smart contract, the operation triggers a revert.

Consequently, all calls to claimAuction fail, preventing the successful claiming of any auction. Note that the attacker only requires 1 wei to initiate the DoS in each auction. Note that all the funds used for bidding in this auction will be stuck in the contract forever, since there is no way of withdrawing them back, even for the admins.

Also, this breaks one of the main invariants of the project, since no one will get their ETH refunded. As indicated in the Main invariants section of the contest page: "The highest bidder will receive the token after an auction finishes, the owner of the token will receive the funds and all other participants will get refunded." Link to the contest page

Tools Used

Manual review.

Recommended Mitigation Steps

To mitigate this vulnerability, it is advisable to utilize pull payments instead of push payments. Allow each user to retrieve their non-winning bids after the auction concludes.

Assessed type

ETH-Transfer

[c4-pre-sort]

141345 marked the issue as duplicate of #843

[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 #2006

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Out of scope

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Out of scope

[xAriextz13]

First of all, thank you for the judging.

I think this issue should be a duplicate of issue #734.

[alex-ppg]

Hey @xAriextz13, this submission is not a duplicate of #734 because the call's result is not validated. As such, the code will continue execution regardless of whether the native transfer was successful. You can try this out yourself in a development environment by issuing a revert instruction on receipt of a native transfer when it is started by an address(foo).call{value:msg.value}("") call.

[xAriextz13]

Sorry for the misunderstand. Thank you!