Malicious user can permanently DOS logic to claimAuction, preventing refunds/payment of fees

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Due to the use of low-level calls in the AuctionDemo:claimAuction, an attacker can fill an auction with a bunch of stink bids, and force any call to claimAuction to run out of gas by wasting the maximum amount of gas in calls to addresses they own. This will prevent actual auctioneers from receiving their refunds, prevent the auction winner from receiving the NFT, and prevent the admin from receiving fees.

Proof of Concept

Consider how the claimAuction function is implemented. Here I have stripped most of the extraneous details & just kept the logic which is done for refunds, which occurs in the else if block:

    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) {
                ...
            } 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 {}
        }
    }

Effectively this function will loop over all auction entries which are not cancelled and send the refund back to the creator of that entry. This ultimately allows an attacker to perform the following attack:

1. Fill the auction for a given tokenId with multiple stink bids (1, 2, 3, ... wei), where the caller is a smart contract with a receive payable function which runs an ~inf loop when triggered

When the auction winner calls claimAuction, this will attempt to pay refunds to all the auctioneers who did not win, which include all the stink bids created by this attacker. On each attempt, the external call to the malicious smart contract will burn the maximum amount of gas possible. Eventually, after attempting to refund a few of these stink bids, this function call will run out of gas, resulting in permanent DOS.

Tools Used

Manual review

Recommended Mitigation Steps

Refunds should be implemented in such a way that users call a separate function to receive their refund, rather than this logic happening in the claimAuction function.

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #1632

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

[c4-judge]

alex-ppg marked the issue as satisfactory