Adversary can block `claimAuction()` due to push-strategy to transfer assets to multiple bidders

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

claimAuction() implements a push-strategy instead of a pull-strategy for returning the bidders funds.

This gives the opportunity for an adversary to DOS the function, locking all funds from other participants.

🤖 This finding differs from the Bots report M-01, and L-18, as it has a different root cause, can't be prevented with any of the mitigations from the bot, and also highlights the actual High impact of the issue.

Impact

• Prevent other bidders from receiving their bids back
• Prevent the token owner from receiving earnings
• Prevent the bidder winner from receiving the NFT

All Ether from the auction can be lost as there is no alternate way of recovering it.

This also breaks one of the main invarians of the protocol:

Properties that should NEVER be broken under any circumstance:

• 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.

Proof of Concept

An adversary can create bids for as little as 1 wei, as there is no minimum limitation: AuctionDemo.sol#L58. With that it can participate in as many auctions as they want to grief all auctions.

All non-winning bidders that didn't cancel their bid before the auction ended will receive their bids back during claimAuction():

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        /// ...
->      for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            } else if (auctionInfoData[_tokenid][i].status == true) {
->              (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

AuctionDemo.sol#L116

The contracts calls the bidders with some value. If the receiver is a contract it can execute arbitrary code.

A malicious bidder can exploit this to make the claimAuction() always revert, and so no funds to other participants be payed back.

With the current implementation, a gas bomb can be implemented to perform the attack. The bot finding L-18 suggests the use of excessivelySafeCall() to prevent it, but it limits the functionality of legit contract receivers, and eventually preventing them from receiving the funds. In addition the finding doesn't showcase the High severity of the issue and may be neglected.

The bot finding M-01 points out the lack of a check of the success of the calls. If the function reverts when the call was not successful, the adversary can make a callback on its contract receive() function to always revert to perform this same attack. In addition this finding also doesn't showcase the High severity of the issue.

Ultimately the way to prevent this attack is to separate the transfer of each individual bidder to a separate function.

Tools Used

Manual Review

Recommended Mitigation Steps

Create a separate function for each bidder to claim their funds back (such as the cancel bids functions are implemented).

Assessed type

ETH-Transfer

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

[c4-judge]

alex-ppg marked the issue as selected for report

[alex-ppg]

The Warden specifies a way in which an auction can be sabotaged entirely in its claim operation by gas griefing.

While the finding has been identified as L-18 in the bot report, it may indeed be neglected and inadequately encompasses the severity of this flaw.

As the Warden clearly illustrates, all funds in the auction will be permanently lost and in an easily accessible fashion by simply either consuming all gas in meaningless operations or gas-bombing via an arbitrarily large payload being returned.

This particular submission was selected as best-for-report due to its acknowledgment of bot issues, precisely & correctly specifying the impact of the exhibit, and providing a PoC that demonstrates the problem.

The C4 Supreme Court verdict did not finalize on issues based entirely on bot findings and as such, I will deem this exhibit a valid high submission given its accessibility and 2-tier upgrade from the bot report (L to H).

[c4-judge]

alex-ppg marked the issue as satisfactory