AI Arena - ni8mare's results

Lines of code

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

Vulnerability details

Impact

Non-transferrable game items can still be transferred using batch transfer.

Proof of Concept

The safeTransferFrom function implements a check to see where the given game item(ERC1155 token) is transferrable using the following check:

    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable);
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

It requires require(allGameItemAttributes[tokenId].transferable) to be true to be transferred. However, this check is not implemented for safeBatchTransferFrom that the ERC1155 standard uses. This allows users to transfer non-transferrable tokens. Thus, breaking the game's logic.

Tools Used

Manual review

Recommended Mitigation Steps

Override the safeBatchTransferFrom function and include the above check for it as well.

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L530-L531

Vulnerability details

Impact

The calculation of the staking factor is incorrect. Malicious users will always exploit it to stake less and win more points.

Proof of Concept

Consider users A and B with the same/similar ELO ratings and assume that they have no stake at risk.

Observe the way the staking factor is calculated:

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

We have assumed that both users have no stake at risk for the sake of this example.

Let's say that the amount staked by user A is 10**18 tokens, whereas user B has staked as little as 1 token. Hence, the stakingFactor_ for user A is 1 and for user B it is calculated as 0, but due to the conditional above it is set to 1 for user B as well.

Now, points are calculated in the following way:

points = stakingFactor[tokenId] * eloFactor;

Note that we have assumed that ELO ratings are the same for both users and since they have the same stakingFactor, the points calculated will also be the same for both users.

This means that a user with 1 token and another with 10**18 tokens staked will earn the same number of points, provided they have the same/similar ELO ratings. And when NRNs are distributed after a round, they are both likely to win the same amount of NRN tokens.

This would be a very unfair system and would not encourage users to stake more NRN tokens.

Tools Used

Manual review

Recommended Mitigation Steps

Change the way the staking factor is calculated or ensure that a minimum amount of stake is needed to earn points.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L379

Vulnerability details

Impact

reRoll function is used to roll a fighter with random traits. But, these generated traits aren't random and thus can be gamed to generate characters with whatever traits a user requires.

Proof of Concept

Note the comment above the reRoll function. It talks about generating random traits.

    /// @notice Rolls a new fighter with random traits.
    /// @param tokenId ID of the fighter being re-rolled.
    /// @param fighterType The fighter type.
    function reRoll(uint8 tokenId, uint8 fighterType) public {

For randomness it uses dna which equals uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId]))). This value is then sent as an input to the _createFighterBase function which creates traits for the character associated with the token id. But, uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId]))) will not generate truly random traits. It's a source of pseudo-randomness. A malicious user already knows the tokenId, numRerolls[tokenId] and can use any address to generate dna values that they want. Then dna value is used to generate traits like element and weight. So, a user can control what traits they want for their character.

Traits like element and weight are quite crucial in the game. This is what the documentation says about weight:

🏋️ Weight - The relative composition of the metal alloy determines a fighter’s weight in the game. A fighter’s weight is the primary determinant of its other relative strength and weaknesses (i.e. all other battle attributes are a function of weight). Additionally, it is used to calculate how far the fighter moves when being knocked back.

If weight can be manipulated, then the game can be manipulated.

Tools Used

Manual review

Recommended Mitigation Steps

Use a true source of randomness like Chainlink VRF.

Assessed type

Other