Any bidder can prevent all other bidders from refunding their bids and a winner from claiming their NFT in claimAuction

[c4-submissions]

Lines of code

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

Vulnerability details

Vulnerability Details

The claimAuction function in AuctionDemo.sol allows users to refund their bids and an auction's winner to claim their won NFT. However, as the function makes an external call on each bidder to refund their bid, the high possibility of a Gas Bomb attack arises.
As each ETH transfer is not assured to have been successful, the external call failing would not revert the call to claimAuction. Despite this, a malicious bidder could perform a Gas Bomb attack the following way: a .call() is made to a bidder address that is a smart contract. The contract returns bytes data with a substantial size that is then copied in memory in the claimAuction function. The size is so great that it consumes the remaining gas making the function revert.
Another way is by exploiting the 1/64 rule of external calls. An ETH transfer would invoke a contract's receive function which would consume all of the 63/64 gas provided. The remaining 1/64 of gas would not be enough to finish the transaction execution, leading to the claimAuction function reverting.

Impact

Both attacks would completely block the winner of an NFT from claiming it and would prevent all bidders from refunding their bids, leading to massive losses for users.
The attacker would practically not suffer any losses as they only need to have bidded in an auction, in order to perform the DOS, which could be done at the auction's start at a price as low as 1 wei.

Proof of Concept

  function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
        auctionClaim[_tokenid] = true;
        uint256 highestBid = returnHighestBid(_tokenid);
        address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
        address highestBidder = returnHighestBidder(_tokenid);
        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 {}
        }
    }

The claimAuction function does not implement the pull-over-push pattern, but instead makes an external call to each bidder's address to refund the bidded ETH.

Tools Used

Manual review

Recommended Mitigation Steps

Implement the pull-over-push method by adding a claim function that allows every bidder to individually refund their bid or claim their won NFT.

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