DoS claimAuction() and cancelAllBids() Can Cause Fund Lockup

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

An attacker can intentionally DoS the claimAuction() function, resulting in a potential fund lockup within AuctionDemo.sol.

Proof of Concept

This vulnerability allows bidders to effectively disable or DoS the claimAuction() function, which can lead to funds becoming stuck in the AuctionDemo.sol. Only auction winners and admins have permission to execute claimAuction(). This function serves the purpose of transferring NFT to the winner while paying the original owner's funds. Additionally, it iterates through auctionInfoStru to refund participants who didn't win.

Here is how the grief/DoS works:

1. The attacker creates a smart contract that lacks an implementation of receive() or fallback().
2. The attacker utilizes this smart contract to participateToAuction().
3. When the auction concludes, the winner or admins attempts to call claimAuction().
4. The claimAuction() function will revert, as the smart contract created by the attacker is incapable of receiving native tokens during the refund process within claimAuction().

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

cancelAllBids() can also be DoS as it has similar refund logic.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider creating a different function to handle refunds rather than everything in claimAuction(). It is likely to run out of gas anyways.

Assessed type

DoS

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

[c4-judge]

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

[Henrychang26]

Thanks for judging @alex-ppg

I believe this submission is incorrectly duped, it should be a valid dupe of #734
The attack mentioned does not need to rely on low-level to revert (or the lack of returned value checked).
Attacker can make claimAuction() always revert by implementing a malicious contract that can not receive native token.
The root cause is not having receive() or fallback() in the malicious contract.

[alex-ppg]

Hey @Henrychang26, thanks for the follow-up! The absence of a receive() or fallback() function would be ignored by the code. You can test this yourself in a development environment, here is some quick code:

contract NoReceipt {
}

contract SendTest {
    function sendFunds() external payable returns (bool) {
        NoReceipt target = new NoReceipt();
        (bool success, ) = address(target).call{value: msg.value}("");
        return success;
    }
}