Forging mechanism can prevent the existence of the "Golden God" entropy in the next generation

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntropyGenerator/EntropyGenerator.sol#L101 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntropyGenerator/EntropyGenerator.sol#L206 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L153 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L181 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L280 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L311 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L345 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntityForging/EntityForging.sol#L102

Vulnerability details

Vulnerability Details

Every generation is supposed to have 1 "Golden God" entropy, which location is set somewhere in the final third of the entropySlots array in EntropyGenerator.sol through the initializeAlphaIndices function:

EntropyGenerator.sol

    //select index points for 999999, triggered each gen-increment
    function initializeAlphaIndices() public whenNotPaused onlyOwner {
        uint256 hashValue = uint256(keccak256(abi.encodePacked(blockhash(block.number - 1), block.timestamp)));

        uint256 slotIndexSelection = (hashValue % 258) + 512;
        uint256 numberIndexSelection = hashValue % 13;

        slotIndexSelectionPoint = slotIndexSelection;
        numberIndexSelectionPoint = numberIndexSelection;

        if (slotIndexSelection == 769 && numberIndexSelection > 3) {
            initializeAlphaIndices();
        }
    }

When minting a new NFT with money (ETH), EntropyGenerator::getNextEntropy function gets called and it progresses towards the "Golden God" entropy:

TraitForgeNft.sol

    function mintToken(bytes32[] calldata proof)
        public
        payable
        whenNotPaused
        nonReentrant
        onlyWhitelisted(proof, keccak256(abi.encodePacked(msg.sender)))
    {
        uint256 mintPrice = calculateMintPrice();
        require(msg.value >= mintPrice, "Insufficient ETH send for minting.");

@>      _mintInternal(msg.sender, mintPrice);

        uint256 excessPayment = msg.value - mintPrice;
        if (excessPayment > 0) {
            (bool refundSuccess,) = msg.sender.call{value: excessPayment}("");
            require(refundSuccess, "Refund of excess payment failed.");
        }
    }

        function _mintInternal(address to, uint256 mintPrice) internal {
        if (generationMintCounts[currentGeneration] >= maxTokensPerGen) {
            _incrementGeneration();
        }

        _tokenIds++;
        uint256 newItemId = _tokenIds;
        _mint(to, newItemId);
@>      uint256 entropyValue = entropyGenerator.getNextEntropy();
        .
        .

    }

However, when breeding/forging 2 NFTs, a new NFT is created from the next generation which doesn't get its entropy from the getNextEntropy function, so there is no progression towards the "Golden God" entropy, but it depends on the entropies of the 2 parents:

EntityForging.sol

    function forgeWithListed(uint256 forgerTokenId, uint256 mergerTokenId)
        external
        payable
        whenNotPaused
        nonReentrant
        returns (uint256)
    {
        .
        .
        .

@>      uint256 newTokenId = nftContract.forge(msg.sender, forgerTokenId, mergerTokenId, "");
        (bool success,) = nukeFundAddress.call{value: devFee}("");
        require(success, "Failed to send to NukeFund");
        (bool success_forge,) = forgerOwner.call{value: forgerShare}("");
        require(success_forge, "Failed to send to Forge Owner");

        // Cancel listed forger nft
        _cancelListingForForging(forgerTokenId);

        uint256 newEntropy = nftContract.getTokenEntropy(newTokenId);

        emit EntityForged(newTokenId, forgerTokenId, mergerTokenId, newEntropy, forgingFee);

        return newTokenId;
    }

TraitForgeNft.sol

    function forge(address newOwner, uint256 parent1Id, uint256 parent2Id, string memory)
        external
        whenNotPaused
        nonReentrant
        returns (uint256)
    {
        require(msg.sender == address(entityForgingContract), "unauthorized caller");
        uint256 newGeneration = getTokenGeneration(parent1Id) + 1;

        /// Check new generation is not over maxGeneration
        require(newGeneration <= maxGeneration, "can't be over max generation");

        // Calculate the new entity's entropy
        (uint256 forgerEntropy, uint256 mergerEntropy) = getEntropiesForTokens(parent1Id, parent2Id);
@>      uint256 newEntropy = (forgerEntropy + mergerEntropy) / 2;

        // Mint the new entity
@>      uint256 newTokenId = _mintNewEntity(newOwner, newEntropy, newGeneration);

        return newTokenId;
    }

There is a maximum of 10,000 NFTs per generation. This means, if for example the "Golden God" entropy gets placed in slot 8,500 and in the previous generation there are more than 1,500 breeds/forges, the "Golden God" entropy won't get minted.

Impact

The "Golden God" entropy will be impossible to get minted in a generation.

Proof of Concept

1. It's generation 1. There are 2,000 forges which means there are 2,000 NFTs minted from generation 2.
2. 10,000 NFTs get minted from generation 1, so it's time to increment generation:

TraitForgeNft.sol

    function _mintInternal(address to, uint256 mintPrice) internal {
        if (generationMintCounts[currentGeneration] >= maxTokensPerGen) {
@>          _incrementGeneration();
        }

    function _mintNewEntity(address newOwner, uint256 entropy, uint256 gen) private returns (uint256) {
        require(generationMintCounts[gen] < maxTokensPerGen, "Exceeds maxTokensPerGen");
        .
        .
        if (generationMintCounts[gen] >= maxTokensPerGen && gen == currentGeneration) {
@>          _incrementGeneration();
        }

    function _incrementGeneration() private {
        require(generationMintCounts[currentGeneration] >= maxTokensPerGen, "Generation limit not yet reached");
        currentGeneration++;
        generationMintCounts[currentGeneration] = 0;
        priceIncrement = priceIncrement + priceIncrementByGen;
@>      entropyGenerator.initializeAlphaIndices();
        emit GenerationIncremented(currentGeneration);
    }

3. The "Golden God" entropy gets placed between slots 8,001-10,000.
4. The "Golden God" entropy can never get minted because there are already 2,000 NFTs from generation 2 and through the EntropyGenerator::getNextEntropy it will never reach that slot, because when the generation reaches 10,000 total NFTs, it will progress to the next generation.

Tools Used

Manual review

Recommended Mitigation Steps

1. Keep track if the "Golden God" entropy has been minted in a generation and if it hasn't and it's the last mint of the generation, force mint the "Golden God" entropy.
2. Keep track of the amount of forges that have happened and don't allow the "Golden God" entropy to be place in an unreachable slot.

Assessed type

Math

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto marked the issue as duplicate of #656

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)