Player can time a Fighter mint to choose physical attribute rarity

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L214-L214 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L324-L324

Vulnerability details

Impact

The physical attributes of Fighter NFTs have rarity, and typically the greater the scarcity, the greater the implied relative value.

FighterFarm has three functions for minting Fighters, and besides the special case for mint passes (that has a limited run), the other function two functions are expected to adhere to the rarities for Fighter physical attributes (using pseudo randomness).

A Player can manipulate the generated value, breaking the randomness invariant, and transitively the scarcity of Fighter physical attributes, by selecting their rarity.

Proof of Concept

The calculation of the physical attribute rarity can be controlled by the Player, by choosing the timing of their Fighter NFT minting to generate the DNA they desire.

Physical attributes have rarity

Each of the Fighter's physical attributes are randomized and has a rarity associated as described in the AI Arena NFT Makeup - The Skin docs

The Skin

The Skin is the outer layer of the NFT, giving it a distinct visual appearance. There are 8 categories of randomized attributes which make up the appearance. Each attribute category has a rarity associated with it.

Categories of Physical Attributes

This DNA sequence is randomly generated off-chain and used to extract the physical attributes. Each generation of fighters has a different set of attributes with a given DNA sequence.

Levels of Rarity

Each physical attribute has a rarity associated with it. There are a total of 4 rarities: Bronze, Silver, Gold, and Diamond.

DNA determines physical attribute rarity

All paths to create a Fighter NFT share the common function FighterFarm::_createNewFighter() that accepts a dna parameter that generate the physical attributes with AiArenaHelper::createPhysicalAttributes() that accepts dna

    function createPhysicalAttributes(
        uint256 dna, 
        uint8 generation, 
        uint8 iconsType, 
        bool dendroidBool
    ) 

Outside the special case of iconsType, dna is used in conjunction with an array of prime numbers to determine rarity

                    uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
                    uint256 attributeIndex = dnaToIndex(generation, rarityRank, attributes[i]);

As the elements of attributeToDnaDivisor are known (public data), when you know the dna value, you can calculate the rarity for each of the physical attributes.

Controlling the DNA

In FighterFarm::claimFighters() and FighterFarm::mintFromMergingPool() the same technique is used to calculate dna

            uint256(keccak256(abi.encode(msg.sender, fighters.length))), 

The Player can calculate the dna value and resulting physical rarities off-chain, as all the data is available.

As the Player controls when the mint function is called, they can simply wait on other Players to create Fighters (and increase fighters.length), until the dna value on their mint will create favourable physical attributes.

Tools Used

Manual review

Recommended Mitigation Steps

Ensure consistent Fighter dna by using input that the receiver cannot manipulate.

Add aew mapping for number of mints for a receiver address, and use that total in combination with the receiver address for the dna in FighterFarm

+    /// @notice Mapping to keep track of the number of Fighter mints fromo each address.
+    mapping(address => uint256) public mintedFightersByAddress;

    function claimFighters(
        uint8[2] calldata numToMint,
        bytes calldata signature,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        // @note `nftsClaimed` used as the claim flag
        bytes32 msgHash = bytes32(keccak256(abi.encode(
            msg.sender, 
            numToMint[0], 
            numToMint[1],
            nftsClaimed[msg.sender][0],
            nftsClaimed[msg.sender][1]
        )));
        require(Verification.verify(msgHash, signature, _delegatedAddress));
        uint16 totalToMint = uint16(numToMint[0] + numToMint[1]);
        require(modelHashes.length == totalToMint && modelTypes.length == totalToMint);
        nftsClaimed[msg.sender][0] += numToMint[0];
        nftsClaimed[msg.sender][1] += numToMint[1];
        for (uint16 i = 0; i < totalToMint; i++) {
+           mintedFightersByAddress[to]++;
            _createNewFighter(
                msg.sender, 
-               uint256(keccak256(abi.encode(msg.sender, fighters.length))), 
+               uint256(keccak256(abi.encode(msg.sender, to, mintedFightersByAddress[to]))), 
                modelHashes[i], 
                modelTypes[i],
                i < numToMint[0] ? 0 : 1,
                0,
                [uint256(100), uint256(100)]
            );
        }
    }

    function mintFromMergingPool(
        address to, 
        string calldata modelHash, 
        string calldata modelType, 
        uint256[2] calldata customAttributes
    ) 
        public 
    {
        require(msg.sender == _mergingPoolAddress);
+       mintedFightersByAddress[to]++;
        _createNewFighter(
            to, 
-            uint256(keccak256(abi.encode(msg.sender, fighters.length))), 
+            uint256(keccak256(abi.encode(msg.sender, to, mintedFightersByAddress[to]))), 
            modelHash, 
            modelType,
            0,
            0,
            customAttributes
        );
    }

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #53

[c4-judge]

HickupHH3 changed the severity to 3 (High Risk)

[c4-judge]

HickupHH3 marked the issue as satisfactory

[c4-judge]

HickupHH3 changed the severity to 2 (Med Risk)

[c4-judge]

HickupHH3 marked the issue as duplicate of #376