`claimAuction` function of `AuctionDemo.sol` will be in DOS and bidders' funds will be stucked. If any bidder's contract revert on receiving ether.

[c4-submissions]

Lines of code

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

Vulnerability details

Summary :

When auction starts for a _tokenid, as the first bidder or by frontrunning the first bidder malicious user will call participateToAuction function of AuctionDemo.sol and he can become the first bidder by passing some wei as bid. After the auction ended winner/admin will call the claimAuction function. But if malicious bidder doesn't want to receive his ethers because he just passed some wei then he can revert from his contract's receive() function. And claimAuction call will fail every time for this _tokenid and claimAuction will be in Denial of Service mode. All the funds of other bidder will be stuck and NFT will not also be transferred to highest bidder.

Vulnerability Details and POC :

1. Malicious user will call participateToAuction function when auction starts for a _tokenid. Because next bid amount need to be greater than previous highest bid amount, so malicious user will enter as first bidder or frontrun the first bidder so that he can enter first with just some dust amount because for first bidder previous highest amount will be 0.

hardhat/smart-contracts/AuctionDemo.sol#L57-L61

57: function participateToAuction(uint256 _tokenid) public payable {
58:        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
59:        auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
60:        auctionInfoData[_tokenid].push(newBid);
      }

2. When the auction ends then Winner/Admin will call claimAuction function. This function should transfer the NFT to highest bidder and to all other bidder their bid amounts. But when claimAuction will try to transfer bid amounts ether (line 116 in below code) to malicious user. His malicious contract will revert and will not accept the ether. Due to which whole transaction will revert and claimAuction will be in DoS mode.

Note : Here at line 116 after low level call return value not checked. It itself is an issue but it is founded in bot report. So by adding that consideration(require(success,"Transfer Failed");) it will be in DoS mode. In current code it won't revert but if malicious user want to revert anyway. He can consume all the gas and due to out gas the whole transaction can still revert.

hardhat/smart-contracts/AuctionDemo.sol##L104-L120

104:  function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
105:        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
106:        auctionClaim[_tokenid] = true;
107:        uint256 highestBid = returnHighestBid(_tokenid);
108:        address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
109:        address highestBidder = returnHighestBidder(_tokenid);
110:        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
111:            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
112:                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
113:                (bool success, ) = payable(owner()).call{value: highestBid}("");
114:                emit ClaimAuction(owner(), _tokenid, success, highestBid);
115:            } else if (auctionInfoData[_tokenid][i].status == true) {
116:                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");//@audit check return value and it will revert if any bidder revert on receiving ether(if bidding through contract),,
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

3. All the funds of all the bidders will be stuck in Auction.sol who bid to purchase that _tokenid NFT. Their is no emergency withdraw function to withdraw ethers if auction ends and claimAuction in DoS mode. User will not be able to cancel bids when auction ends.

Malicious bidder Contract

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./IAuctionDemo.sol";

contract MaliciousBidder{
        constructor(IAuctionDemo demo, uint256 _tokenid) payable{
                demo.participateToAuction{value : 50}(_tokenid); //entering with 50 wei
        }

        receive() external payable {
          revert(); //will always revert when ether will be transferred to this contract except first time when constructor runs
        }
}

Note : Their is DoS in bot report also for this function also but thier root cause is completely different, That was due to array out of bounds but here the root cause is completely different

Impact :

All the funds of all the bidders will be stuck in Auction.sol who bid to purchase that _tokenid NFT. if auction ends and claimAuction in DoS mode no way to withdraw ethers of bidders who bid to purchase that _tokenid. User will not be able to cancel bids also when auction ends.

Tools Used :

Manual Review

Recommended Mitigation :

1. Use pull over push strategy to transfer ethers to users addresses. So instead of transferring their bid amount ethers in same txn to all bidders. Let them withdraw by themselves.
2. Make a withdraw function for them who is not highest bidder. So that they can withdraw their ethers by themselves after auction ends.

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 #2006

[c4-judge]

alex-ppg marked the issue as selected for report

[alex-ppg]

The Warden specifies that, after remediation for a bot finding is applied, the recipients of bid refunds could hijack the call chain of the claimAuction function to revert.

Firstly, this submission falls under the Speculation on Future Code Supreme Court Verdict whereby they attempt to justify why the Sponsor would make the necessary code change for native transfers that would result in this vulnerability. Specifically, they attempt to justify that remediation for M-01 would result in the vulnerability manifesting.

The Supreme Court ruling specifically states that the root cause should be present in the code already. As the Warden relies on an assumption of potential remediation rather than future code that inherits from/integrates with the existing in-scope codebase, I consider this exhibit to be out-of-scope.

To note, some duplicates of this issue simply refer to revert being sufficient in the current implementation for causing the DoS which is invalid, as the success boolean is not evaluated in any shape or form meaning that a revert operation would be ignored and the code would successfully continue its execution.

[c4-judge]

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

[c4-judge]

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