search

code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

Critical Issues in Auction Refund Loop #1974

Closed c4-submissions closed 7 months ago

c4-submissions commented 7 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/tree/main/smart-contracts/AuctionDemo.sol#L116

Vulnerability details

Impact

The vulnerability in the implementation of the refund loop within the AuctionDemo contract has several critical impacts that need to be addressed. The root cause of the vulnerability is that the refund process, when combined with the claimAuction function, results in multiple intertwined issues:

  1. Out-of-Gas Risk: The contract's large auctionInfoData[id] array makes it likely to run out of gas during the execution of the refund loop. This could prevent users from claiming their refunds and significantly disrupt the functioning of the auction contract.

  2. Stuck Funds: It's expected that all funds should be refunded. However, due to a lack of proper success return value checks, there is a risk of funds getting stuck in the contract. If claimAuction is called but fails without revert at Line 116, it would leave funds locked in the contract since claimAuction can only be called once, and there's no mechanism for emergency withdrawal of the ETH. The Bot Race did report this finding as M-01

  3. Malicious Participant Lockout: Fixing the issue no.2 via reverting on failed transfer would create a new vulnerability. A malicious participant contract can decide to revert when receiving an ETH refund in their fallback function. This can effectively lock the entire funds and the auctioned NFT. As a result, the winner cannot claim their NFT, the contract owner cannot receive the highest bid payment, and other participants cannot get their refunds.

Proof of Concept

To illustrate this vulnerability, consider a scenario in which Alice participates in an auction via her contract. When Bob wins and attempts to claim the auction, the refund loop is executed, but Alice's contract maliciously reverts in its fallback function, leading to the locking of funds and the auctioned NFT.

  1. Alice participates in an auction via her contract, which can call claimAuction if she wins but can also deny the refund in her fallback function if she loses.
  2. Bob wins the auction and attempts to claim the auction.
  3. The refund loop is executed to refund participants, including Alice, via payable(Alice contract).call{value: bid}.
  4. Alice's contract reverts in its fallback function due to her malicious intention.
  5. The entire auction pot and the NFT get locked because the claimAuction function consistently fails due to Alice's behavior.

The severity of these issues is extremely high, as they can lead to funds being permanently locked in the contract, the inability to claim auctioned NFTs, and potential disruption of the auction's integrity.

Tools Used

Manual Review

Recommended Mitigation Steps

To address these critical issues, it is recommended to separate the refund claim process from the claimAuction function. This separation would allow each user to individually claim their own ETH refund without the risk of blocking other users. Additionally, proper checks for the success return value should be implemented to ensure that funds are not stuck in the contract.

Assessed type

ETH-Transfer

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #486

c4-judge commented 7 months ago

alex-ppg marked the issue as not a duplicate

alex-ppg commented 7 months ago

The Warden has marked multiple issues that are not necessarily intertwined as the Warden specifies.

The Out-of-Gas issue is a duplicate of #2038 albeit with little mention.

The Stuck Funds issue is indeed the M-01 finding of the bot report.

The Malicious Participant Lockout arises from a potential remediation for M-01 rather than the actual code in the scope of the contest. To note, there are multiple ways to "correct" M-01 and the bot does not specify an alleviation; it merely points out the issue. As an example, the code could validate the success flag and store a refund to-be-claimed in a mapping, along with a myriad other potential remediation paths that would satisfy the bot's criteria (checking success) without necessarily reverting on error.

As such, I consider this exhibit out-of-scope given that even if it is considered a duplicate of #2038, that exhibit is also a bot-report finding.

c4-judge commented 7 months ago

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