AI Arena - dipp's results

Lines of code

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

Vulnerability details

Impact

A fighter/token may be unstaked while it has a non-zero stakeAtRisk amount allowing the owner of the token to still receive rewards based on their stakeAtRisk amount.

Additionally, a malicious user may transfer the token to an address that has amountLost = 0 pn the StakeAtRisk contract which would cause the updateBattleRecord function to revert if the token wins due to an underflow in StakeAtRisk:reclaimNRN.

Proof of Concept

When a token/fighter loses a battle and their points are 0, a portion of the stake on the token is sent to the StakeAtRisk contract. The token owner may unstake from RankedBattle to receive the remainder of the stake and make the token transferable in the round again.

The updateBattleRecord may still be called for tokens that have been unstaked for the round resulting in either (1) increase in points and potentially reclaiming stakeAtRisk if winning or (2) decrease in points if the token has gained points from wins after unstaking.

The test code below shows that the batlle record may be updated for a token that is not staked but has stakeAtRisk:

function testUpdateBattleRecordEvenIfNotStaked() public {
        address player = vm.addr(3);
        _mintFromMergingPool(player);
        uint8 tokenId = 0;
        _fundUserWith4kNeuronByTreasury(player);
        vm.prank(player);
        _rankedBattleContract.stakeNRN(3_000 * 10 ** 18, 0);
        assertEq(_rankedBattleContract.amountStaked(0), 3_000 * 10 ** 18);
        uint256 curStakeAtRisk = (_rankedBattleContract.bpsLostPerLoss() * _rankedBattleContract.amountStaked(0)) / 10**4;
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        (,, uint256 losses) = _rankedBattleContract.fighterBattleRecord(tokenId);
        assertEq(losses, 1);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(tokenId), curStakeAtRisk);

        // Mint a fighter to the second user so that they can lose a battle and have stakeAtRisk to prevent underflow in StakeAtRisk:reclaimNRN
        address player2 = vm.addr(4);
        _mintFromMergingPool(player2);
        _fundUserWith4kNeuronByTreasury(player2);
        vm.prank(player2);
        _rankedBattleContract.stakeNRN(3_000 * 10 ** 18, 1);
        assertEq(_rankedBattleContract.amountStaked(1), 3_000 * 10 ** 18);
        curStakeAtRisk = (_rankedBattleContract.bpsLostPerLoss() * _rankedBattleContract.amountStaked(1)) / 10**4;
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(1, 50, 2, 1500, true);
        (,, losses) = _rankedBattleContract.fighterBattleRecord(1);
        assertEq(losses, 1);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(1), curStakeAtRisk);


        // Unstake figher 0 as the first user and transfer to the second user
        vm.startPrank(player);
        _rankedBattleContract.unstakeNRN(type(uint256).max, 0);
        _fighterFarmContract.transferFrom(player, player2, 0);
        vm.stopPrank();

        // The game server may still update the record for a fighter that is not staked but has stakeAtRisk
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 0, 1500, true);
        (uint256 wins,,) = _rankedBattleContract.fighterBattleRecord(0);
        assertEq(wins, 1);
        assertGt(_rankedBattleContract.accumulatedPointsPerAddress(player2, _rankedBattleContract.roundId()), 0);     // The player can still earn rewards on fighter 0 even though it is not staked.
    }

Tools Used

Recommended Mitigation Steps

Consider checking the hasUnstaked bool in the updateBattleRecord to prevent updating for unstaked tokens.

Assessed type

Other