AI Arena - n0kto's results

Lines of code

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

Vulnerability details

Description

The protocol intended to have non-transferable items, and to implement that, they added a requirement using a boolean attribute in the overriding transferFrom function. However, the ERC1155 standard includes a function called safeBatchTransferFrom to transfer multiple items simultaneously, and this function is not overridden. Using this function, any object can be transferred, bypassing the transferable attribute of items.

Impact

Likelihood: High

• Can be used at any time by any item owner or on any items with an approval.

Impact: High

• All the "transferability" rights for an item are bypassable.

Proof of Concept

Foundry PoC to add in GameItems.t.sol

    uint[] private ids;
    uint[] private amount;

    function testTransferAllItemsWithoutRights() public {
        _gameItemsContract.createGameItem(
            "Turbo",
            "https://ipfs.io/ipfs/",
            true,
            false, // transferable = false !!
            10_000,
            1 * 10 ** 18,
            10
        );
        (string memory name, , bool transferable, , , ) = _gameItemsContract
            .allGameItemAttributes(1);
        assertEq(name, "Turbo");
        assertFalse(transferable);

        // Create a new player and a receiver
        address player = makeAddr("player");
        address receiver = makeAddr("receiver");

        _fundUserWith4kNeuronByTreasury(player);

        vm.startPrank(player);
        // Player buy batteries which are not transferable.
        _gameItemsContract.mint(1, 2);
        ids.push(1);
        amount.push(2);

        // Player transfers them using safeTransferBatch
        _gameItemsContract.safeBatchTransferFrom(
            player,
            receiver,
            ids,
            amount,
            ""
        );

        // Receiver has received the non-transferable object
        assertEq(_gameItemsContract.balanceOf(receiver, 1), 2);
        vm.stopPrank();
    }

Recommended Mitigation

Override the _beforeTokenTransfer and/or _afterTokenTransfer functions to properly check all transferable attributes.

A possible solution:

    function _beforeTokenTransfer(
        address operator,
        address from,
        address to,
        uint256[] memory ids,
        uint256[] memory amounts,
        bytes memory data
    ) internal view override {
        for (uint256 i; i < ids.length; i++)
            require(
                allGameItemAttributes[ids[i]].transferable,
                "At least one item is not transferable"
            );
    }

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/MergingPool.sol#L139-L144

Vulnerability details

Description

MergingPool::claimRewards allows any user who wins in the MergingPool to mint a new fighter. The issue is that winners can set arbitrary models and attributes themselves. Winners who want specific attributes can set them for their new fighter, thanks to a special condition in FighterFarm::_createNewFighter for MergingPool winners. Winners who see a fighter better than theirs can check the blockchain for the AI model of that fighter and inject it into their next fighter.

    function claimRewards(
@>        string[] calldata modelURIs,
@>        string[] calldata modelTypes,
@>        uint256[2][] calldata customAttributes
    ) external {
        ...
    }

    function _createNewFighter(...) private {
        require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);
        uint256 element;
        uint256 weight;
        uint256 newDna;
        if (customAttributes[0] == 100) {
            (element, weight, newDna) = _createFighterBase(dna, fighterType);
@>        } else {
@>            element = customAttributes[0];
@>            weight = customAttributes[1];
@>            newDna = dna;
        }

Impact

Likelihood: High

• Any winners in MergingPool can inject attributes and the model they want into their new fighter.

Impact: High

• Breaks the "imitation" and "skilled" aspect of the game.
• Prevents any possibility for researchers to sell their models.
• A player will choose rare attributes, which is unfair for other players.

Proof of Concept

• Find a fighter with a good ELO
• Check the blockchain for the linked model to that fighter
• Inject the model into the new fighter

The already present test below already proves the injection possibility:

function testClaimRewardsForWinnersOfMultipleRoundIds() public {
    _mintFromMergingPool(_ownerAddress);
    _mintFromMergingPool(_DELEGATED_ADDRESS);
    assertEq(_fighterFarmContract.ownerOf(0), _ownerAddress);
    assertEq(_fighterFarmContract.ownerOf(1), _DELEGATED_ADDRESS);
    uint256[] memory _winners = new uint256[](2);
    _winners[0] = 0;
    _winners[1] = 1;
    // Winners of roundId 0 are picked
    _mergingPoolContract.pickWinner(_winners);
    assertEq(_mergingPoolContract.isSelectionComplete(0), true);
    assertEq(
        _mergingPoolContract.winnerAddresses(0, 0) == _ownerAddress,
        true
    );
    // Winner matches ownerOf tokenId
    assertEq(
        _mergingPoolContract.winnerAddresses(0, 1) == _DELEGATED_ADDRESS,
        true
    );
    string[] memory _modelURIs = new string[](2);
    _modelURIs[
        0
    ] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
    _modelURIs[
        1
    ] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
    string[] memory _modelTypes = new string[](2);
    _modelTypes[0] = "original";
    _modelTypes[1] = "original";
    uint256[2][] memory _customAttributes = new uint256[2][](2);
    _customAttributes[0][0] = uint256(1);
    _customAttributes[0][1] = uint256(80);
    _customAttributes[1][0] = uint256(1);
    _customAttributes[1][1] = uint256(80);
    // Winners of roundId 1 are picked
    _mergingPoolContract.pickWinner(_winners);
    // Winner claims rewards for previous roundIds
    _mergingPoolContract.claimRewards(
        _modelURIs,
        _modelTypes,
        _customAttributes
    );
    // Other winner claims rewards for previous roundIds
    vm.prank(_DELEGATED_ADDRESS);
    _mergingPoolContract.claimRewards(
        _modelURIs,
        _modelTypes,
        _customAttributes
    );
    uint256 numRewards = _mergingPoolContract.getUnclaimedRewards(
        _DELEGATED_ADDRESS
    );
    emit log_uint(numRewards);
    assertEq(numRewards, 0);
}

Recommended Mitigation

Only a trusted role or the game server should create fighters: add a require statement to prevent any user from calling this function by themselves. Alternatively, a solution to avoid paying gas and let users claim the reward by themselves: Put in place a system with a signature from the game server like FighterFarm::claimFighters, but including AI models in the signed data.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Description

In FighterFarm::redeemMintPass, anyone with a pass can create a new fighter. The problem is that fighterTypes and AI models can be set arbitrarily, allowing users to mint specific attributes or even a dendroid (rare fighter) with the best AI model in the game.

    function redeemMintPass(
        uint256[] calldata mintpassIdsToBurn,
@>        uint8[] calldata fighterTypes,
@>        uint8[] calldata iconsTypes,
@>        string[] calldata mintPassDnas,
@>        string[] calldata modelHashes,
@>        string[] calldata modelTypes
    ) external {
        ...
        for (uint16 i = 0; i < mintpassIdsToBurn.length; i++) {
            ...
            _createNewFighter(
                msg.sender,
                uint256(keccak256(abi.encode(mintPassDnas[i]))),
                modelHashes[i],
                modelTypes[i],
@>                fighterTypes[i],
                iconsTypes[i],
                [uint256(100), uint256(100)]
            );
        }
    }

Impact

Likelihood: Medium/High

• Each time a user has a pass.

Impact: High

• Creation of dendroids (rare fighters) for every pass.
• A player can set fighterTypes to 1 which can cause unexpected behavior if the fighter is not a dendroid (every attribute indexes are set to 0 or 1 in dnaToIndex).
• A player can use iconTypes to unlock limited items.
• A player will look for the best competitor, check their AI model on-chain, and inject it into the new fighters.
• Breaks the "imitation" and "skilled" aspect of the game.
• Prevents any possibility for researchers to sell their models since players can use them without paying.

Proof of Concept

The below test in FighterFarm.t.sol already shows the possible injection. I added some comments in it.

    function testRedeemMintPass() public {
        uint8[2] memory numToMint = [1, 0];
        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        // first i have to mint an nft from the mintpass contract
        assertEq(_mintPassContract.mintingPaused(), false);
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
        assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
        assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

        // once owning one i can then redeem it for a fighter
        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] = 0; // Just have to put 1 here to create a dendroid
        _neuralNetHashes[0] = "neuralnethash"; // Choose the best one known this days.
        _modelTypes[0] = "original"; // Choose the best one known this days.
        _iconsTypes[0] = 1; // Choose special attributs you like

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

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

        // check balance to see if we successfully redeemed the mintpass for a fighter
        assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 1);
        // check balance to see if the mintpass was burned
        assertEq(_mintPassContract.balanceOf(_ownerAddress), 0);
    }

Recommended Mitigation

Only a trusted role or the game server should create fighters: add a require statement to prevent any users from calling this function by themselves. Alternatively, a solution to avoid paying gas and let users claim the fighter by themselves: Put in place a system with a signature from the game server like FighterFarm::claimFighters, but all parameters in the signed data.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Description

The FighterFarm::reRoll function allows users to reroll any fighter to have a new DNA, new element, new weights, etc. However, fighterType can be arbitrarily set in the parameters of the function. It could be used to have more rerolls if one type of fighter has more allowed rerolls. If the generation number of the fighter type is not set in the mapping AiArenaHelper::attributeProbabilities, as there is no checker, all attribute indexes will be set to 0, which is not planned by the protocol and can lead to unexpected behavior on the game server. If a generation exists and the fighter type is set to 1 (like a dendroid), _createFighterBase will return a newDna set to 1, which will set rarityRank to 0 in AiArenaHelper::createPhysicalAttributes for all attribute due to the round-down division and will set all attribute indexes to 1.

@>    function reRoll(uint8 tokenId, uint8 fighterType) public {
        require(msg.sender == ownerOf(tokenId));
@>        require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]);
        require(
            _neuronInstance.balanceOf(msg.sender) >= rerollCost,
            "Not enough NRN for reroll"
        );

        _neuronInstance.approveSpender(msg.sender, rerollCost);
        bool success = _neuronInstance.transferFrom(
            msg.sender,
            treasuryAddress,
            rerollCost
        );
        if (success) {
            numRerolls[tokenId] += 1;
            uint256 dna = uint256(
                keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId]))
            );
            (
                uint256 element,
                uint256 weight,
                uint256 newDna
@>            ) = _createFighterBase(dna, fighterType);
            fighters[tokenId].element = element;
            fighters[tokenId].weight = weight;
            fighters[tokenId].physicalAttributes = _aiArenaHelperInstance
                .createPhysicalAttributes(
                    newDna,
@>                    generation[fighterType],
                    fighters[tokenId].iconsType,
                    fighters[tokenId].dendroidBool
                );
            _tokenURIs[tokenId] = "";
        }
    }

    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

Likelihood: High

• Anytime attackers have enough NRN to reroll.

Impact: Medium

• Can choose the highest number of rerolls possible between dendroid and champions depending on the one with more reroll ticket.
• Can set all attributes to the first index of each one or to 0 in some cases, which will lead to unexpected behavior on the game server.

Proof of Concept

• Reroll for a fighter with the fighterType set to 1.
• Check the new attribute indexes of this fighter.

Recommended Mitigation

If a fighter cannot be transformed in dendroid: instead of passing fighterType in the parameters function, use fighters[tokenId].dendroidBool and remove fighterType parameter. Otherwise, game server can sign a message like in FighterFarm::claimFighters. Additionally, consider implementing a check to know if a generation for a specific fighterType exists and revert.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Description

The BattleRanked contract allows people to stake NRN and participate in ranked battles. However, there is no real minimum amount required, except above 0. It means any user putting dust like 1e0 NRN will compete and earn points. But the real problem is that _getStakingFactor will return 1 for any initial amount of stake under 1e18! It means that a people with 1e18 NRN and 1e0 will earn the same number of points (which depends on the eloFactor). Moreover, the calculation of curStakeAtRisk will be biased because of the round-down division per 1e4, which will give 0 if the stake amount is lower than 1e3 (it is multiplied by bpsLostPerLoss which is currently 10).

    function _getStakingFactor(
        uint256 tokenId,
        uint256 stakeAtRisk
    ) private view returns (uint256) {
@>        uint256 stakingFactor_ = FixedPointMathLib.sqrt(
@>            (amountStaked[tokenId] + stakeAtRisk) / 10 ** 18
@>        );
@>        if (stakingFactor_ == 0) {
@>            stakingFactor_ = 1;
@>        }
        return stakingFactor_;
    }

function stakeNRN(uint256 amount, uint256 tokenId) external {
@>        require(amount > 0, "Amount cannot be 0");
        ...
    }

    function updateBattleRecord(...) external {
        ...
@>        if (amountStaked[tokenId] + stakeAtRisk > 0) {
            _addResultPoints(
                battleResult,
                tokenId,
                eloFactor,
                mergingPortion,
                fighterOwner
            );
        }
        ...
    }

function _addResultPoints(...) private {
        ...
@>        curStakeAtRisk =
@>            (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) /
@>            10 ** 4;
        ...

Impact

Likelihood: High

• Any players staking less than 1e18 NRN and more than 0

Impact: High

• Players will get points as if they have staked 1e18 NRN.
• Players will never lose any stakes.
• Players will be able to claim rewards at the end of the round without taking any risk.

Proof of Concept

Foundry PoC to add in RankedBattle.t.sol

    function testUpdateBattleRecordStakingDust() public {
        address player = vm.addr(3);
        _mintFromMergingPool(player);
        uint8 tokenId = 0;
        _fundUserWith4kNeuronByTreasury(player);

        // Player stakes litteraly nothing : 1e0
        vm.prank(player);
        _rankedBattleContract.stakeNRN(1, 0);
        assertEq(_rankedBattleContract.amountStaked(0), 1);
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 0, 0, 1500, true);
        _rankedBattleContract.setNewRound();

        // Player get the same point than anyone putting 1 NRN (1e18) !
        assertEq(_rankedBattleContract.accumulatedPointsPerFighter(0, 0), 1500);
        (uint256 wins, , ) = _rankedBattleContract.fighterBattleRecord(tokenId);
        assertEq(wins, 1);
    }

Recommended Mitigation

Put a real minimum, like 1 NRN (1e18), to participate in RankedBattle.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L169-L186 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L462-L474 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L484-L531

Vulnerability details

Description

Pseudo-randomness for creating fighters are calculated on-chain. This pseudo-randomness is really easy to predict; it only uses constant and predictable values. Some variables are even manipulable by users to increase their chances of having a better fighter.

Below are functions with parts of the pseudo-randomness and functions which call them.

Function used to generate Fighter with pseudo-randomness:

    function _createNewFighter(...) private {
        ...
        if (customAttributes[0] == 100) {
@>            (element, weight, newDna) = _createFighterBase(dna, fighterType);
        }
        ...

@>        FighterOps.FighterPhysicalAttributes memory attrs = _aiArenaHelperInstance
@>            .createPhysicalAttributes(
@>                newDna,
@>                generation[fighterType],
@>                iconsType,
@>                dendroidBool
@>            );
        ...
    }

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

    function createPhysicalAttributes(...) external view returns (FighterOps.FighterPhysicalAttributes memory) {
        ...
        else {
            ...
            for (uint8 i = 0; i < attributesLength; i++) {
                ...
                else {
@>                    uint256 rarityRank = (dna /
@>                        attributeToDnaDivisor[attributes[i]]) % 100;
@>                    uint256 attributeIndex = dnaToIndex(
@>                        generation,
@>                        rarityRank,
@>                        attributes[i]
@>                    );
@>                    finalAttributeProbabilityIndexes[i] = attributeIndex;
                }
            }
            ...
        }
    }

    function dnaToIndex(
        uint256 generation,
        uint256 rarityRank,
        string memory attribute
    ) public view returns (uint256 attributeProbabilityIndex) {
@>        uint8[] memory attrProbabilities = getAttributeProbabilities(
@>            generation,
@>            attribute
@>        );

@>        uint256 cumProb = 0;
@>        uint256 attrProbabilitiesLength = attrProbabilities.length;
@>        for (uint8 i = 0; i < attrProbabilitiesLength; i++) {
@>            cumProb += attrProbabilities[i];
@>            if (cumProb >= rarityRank) {
@>                attributeProbabilityIndex = i + 1;
@>                break;
@>            }
@>        }
@>        return attributeProbabilityIndex;
    }

Creation functions callable by players with manipulable parameters:

    function claimFighters(...) external {
        ...
        for (uint16 i = 0; i < totalToMint; i++) {
            _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)]
@>            );
        }
    }

    function mintFromMergingPool(...) public {
        require(msg.sender == _mergingPoolAddress);
@>        _createNewFighter(
@>            to,
@>             uint256(keccak256(abi.encode(msg.sender, fighters.length))),
@>            modelHash,
@>            modelType,
@>            0,
@>            0,
@>            customAttributes
@>        );
    }

    function reRoll(uint8 tokenId, uint8 fighterType) public {
        ...
        if (success) {
            ...
@>            uint256 dna = uint256(
@>                keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId]))
@>            );
@>            (
@>                uint256 element,
@>                uint256 weight,
@>                uint256 newDna
@>            ) = _createFighterBase(dna, fighterType);
@>            fighters[tokenId].element = element;
@>            fighters[tokenId].weight = weight;
@>            fighters[tokenId].physicalAttributes = _aiArenaHelperInstance
@>                .createPhysicalAttributes(
@>                    newDna,
                    ...
                );
            ...
        }
    }

Impact

Likelihood: Medium/High

• One player can create a simulator with all parameters he can modify, or just use the Solidity code in Foundry tests and make several tries.
• Can be used for any Fighter creation/reroll with variable parameters: FighterFarm::claimFighters, FighterFarm::mintFromMergingPool, FighterFarm::reRoll.

Impact: High

• Can simulate and play with parameters to mint/reroll a fighter with rare or better attributes.
• The creation/reroll fighter would be unfair.

Proof of Concept

• Implement a simulator in any language or use directly Foundry tests to try different combinations with this parameter:
  • for FighterFarm::reRoll: msg.sender (creating new wallet addresses), tokenId (among your fighters), numRerolls[tokenId]
  • for FighterFarm::mintFromMergingPool: msg.sender (creating new wallet addresses), fighter.length (waiting a specific length)
  • for FighterFarm::claimFighters: msg.sender (creating new wallet addresses), fighter.length (waiting a specific length)
• Mint/reRoll to get the best fighter according to your prediction

Recommended Mitigation

Use an oracle like Chainlink to not let people predict randomness and use it to their advantage. Alternatively, send the DNA variable from the game server, don't let players indirectly manipulate it.

Assessed type

Other