NextGen - MaNcHaSsS's results

Lines of code

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

Vulnerability details

Impact

Any auction could be manipulated and disrupted by placing a very low initial bid. Subsequently, the attacker places a really high bid and/or consistently front-run bids, dissuading other bidders from participating. Just before the auction concludes, all but the first attacker's bids are canceled and refunded, enabling the attacker to win the auction with the initially low bid, incurring minimal costs, primarily limited to gas expenses.

The current implementation of cancelAllBids() and cancelBid() allows for the cancellation of any bid, even if it happens to be the winning bid:

    function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
        ...

    function cancelAllBids(uint256 _tokenid) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
          ...

Proof of Concept

Modify your fixtures hardhat/scripts/fixturesDeployment.js to add a 4th addr:

const { ethers } = require("hardhat");

// Setup test environment:
const fixturesDeployment = async () => {
  const signersList = await ethers.getSigners();
  const owner = signersList[0];
  const addr1 = signersList[1];
  const addr2 = signersList[2];
  const addr3 = signersList[3];
  const addr4 = signersList[4];

  const signers = {
    owner: owner,
    addr1: addr1,
    addr2: addr2,
    addr3: addr3,
    addr4: addr4,
  };
   ...

Use this modified nextGen.test.js that:

1. Sets the collection
2. Mints and Auctions an NFT to addr2
3. Bids with a low bid as addr3 intended to win
4. Places a blocking high bid with addr4, attacker will keep placing high bids to block other users
5. Cancels and gets refunded for all addr4 bids before auction ends
6. Checks thats addr3 wins the auction with the low bid

Tools Used

vs code, hardhat

Recommended Mitigation Steps

Modify the cancelAllBids() and cancelBid() to check that the highest bid is not canceled:

    function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        require(auctionInfoData[_tokenid][index].bid < returnHighestBid(_tokenid), "Can not cancel HighestBid");

    function cancelAllBids(uint256 _tokenid) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        uint256 highestBid = returnHighestBid(_tokenid);
        for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true && auctionInfoData[_tokenid][index].bid < highestBid) {
                auctionInfoData[_tokenid][i].status = false;
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
            } else {}
        }
    }

Additionally, I would like to suggest incorporating a minBidIncreasePercentage check to ensure that a new bid is a certain percentage larger than the last high bid. This measure is intended to discourage sniping by 1 wei or front-runners.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

The current state of the AuctionDemo handles bids and claiming an auction in a way that allows a griefing party to permanently lock all non-canceled bids using a griefing attack by forcing the claiming transaction out of gas. This attack could be as inexpensive as the attacker desires, given the absence of a minimum bid check in the current implementation of the auction.

The claimAuction() function iterates through non-canceled bids, refunds bidders, and transfers the auctioned NFT and the winning bid to the owner. However, a malicious contract can disrupt the process when funds are transferred to it.

The current claimAuction() function:

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

Proof of Concept

The vulnerability arises when a griefing contract gets refunded on a placed bid. This happens because the griefing contract receive() intentionally runs out of gas when its being refunded using an infinite loop.

To reproduce this, add the next contract and interface at hardhat/smart-contracts/Griefer.sol and hardhat/smart-contracts/IAuction.sol

pragma solidity ^0.8.2;

interface IAuction {
    /**
     * @dev Participate in the auction by placing a bid on the specified token.
     * @param _tokenid The ID of the token in the auction.
     */
    function participateToAuction(uint256 _tokenid) external payable;
}

pragma solidity ^0.8.2;

import "./IAuction.sol";

contract Griefer {
    // Function to call participateToAuction and send msg.value
    function callParticipateToAuction(
        address _auctionContractAddress,
        uint256 index
    ) public payable {
        IAuction auction = IAuction(_auctionContractAddress);
        auction.participateToAuction{value: msg.value}(index);
    }

    // Fallback function that runs ot of gas
    receive() external payable {
        uint i = 0;
        while (true) {
        }
    }

}

Modify your fixtures hardhat/scripts/fixturesDeployment.js to add the griefer contract

const { ethers } = require("hardhat");

// Setup test environment:
const fixturesDeployment = async () => {
  const signersList = await ethers.getSigners();
  const owner = signersList[0];
  const addr1 = signersList[1];
  const addr2 = signersList[2];
  const addr3 = signersList[3];
  const addr4 = signersList[4];

  ...

  const nextGriefer = await ethers.getContractFactory("Griefer");
  const hhGriefer = await nextGriefer.deploy();

  const contracts = {
    hhAdmin: hhAdmin,
    hhCore: hhCore,
    hhDelegation: hhDelegation,
    hhMinter: hhMinter,
    hhAuction: hhAuction,
    hhRandomizer: hhRandomizer,
    hhRandoms: hhRandoms,
    hhGriefer: hhGriefer,
  };

  const signers = {
    owner: owner,
    addr1: addr1,
    addr2: addr2,
    addr3: addr3,
    addr4: addr4,
  };
   
   ...

Finally you can run the test with the next modified nextGen.test.js, which:

1. Sets an auction
2. Sets some legal bids
3. Bids as the griefer contract
4. Places winning bid
5. Checks for revert out of gas when the winner tries to claim.

Tools Used

vs code, hardhat

Recommended Mitigation Steps

Allowing the retrieval of any non-winning bids at all times and simplifying the claimAuction() function to handle only the winning bid.

    function cancelBid(uint256 _tokenid, uint256 index) public {
        // 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);
    }

    function cancelAllBids(uint256 _tokenid) public {
        // require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
                auctionInfoData[_tokenid][i].status = false;
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
            } else {}
        }
    }

    // claim Token After Auction

    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
                );
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            }
        }
    }

It should be noted that this only resolves the issue in the scope of the griefer attack; the claiming functions in the current state still allow another kind of DoS attack, which has been dealt with in a separate issue.

Assessed type

DoS