AI Arena - merlin's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L342 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L476-L478 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L491-L498

Vulnerability details

Bug Description

In the RankedBattle smart contract, there is an updateBattleRecord function designed to update the battle record for a fighter. If a fighter does not have any points for this round, NRNs are at risk of being lost. After a user loses all staked NRN tokens and they all transfer to the StakeAtRisk smart contract, they can still participate in battles without risking the loss of points or NRN tokens when they lose.

Proof-of-Concept

When the updateBattleRecord function is called, the internal function _addResultPoints is invoked if the condition amountStaked[tokenId] + stakeAtRisk > 0 is met: RankedBattle.sol#L341-L344

uint256 stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);
        if (amountStaked[tokenId] + stakeAtRisk > 0) {
            _addResultPoints(
                battleResult,
                tokenId,
                eloFactor,
                mergingPortion,
                fighterOwner
            );
        }

The smart contract ensures that users cannot lose more NRNs than they have staked in their staking pool: RankedBattle.sol#L472-L478

} else if (battleResult == 2) {
            /// If the user lost the match

            /// Do not allow users to lose more NRNs than they have in their staking pool
            if (curStakeAtRisk > amountStaked[tokenId]) {
                curStakeAtRisk = amountStaked[tokenId];
            }

So when the user has amountStaked = 0, stakeAtRisk > 0, then the current curStakeAtRisk = 0, which means the user will not lose any NRN tokens: RankedBattle.sol#L491-L498

 } else {
                /// If the fighter does not have any points for this round, NRNs become at risk of being lost
                bool success = _neuronInstance.transfer(_stakeAtRiskAddress, curStakeAtRisk);
                if (success) {
                    _stakeAtRiskInstance.updateAtRiskRecords(curStakeAtRisk, tokenId, fighterOwner);
                    amountStaked[tokenId] -= curStakeAtRisk;
                }
            }

The following test case illustrates the scenario:

function testPlayMatchWithZeroAmountStake() public {
        address player = vm.addr(3);
        _mintFromMergingPool(player);
        _fundUserWith4kNeuronByTreasury(player);
        vm.prank(player);
        // stakes 1_000 NRN token
        _rankedBattleContract.stakeNRN(1_000 * 10 ** 18, 0);

        // loses battle and 1 NRN token
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(0), 1 * 10 ** 18);

        // unstakes 999 NRN tokens
        vm.prank(player);
        _rankedBattleContract.unstakeNRN(999 * 10 ** 18, 0);

        // The user participates in battles while having amountStaked - 0,
        // meaning they are not risking anything
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(0), 1 * 10 ** 18);
    }

Impact

The user will not lose points or NRN tokens when all their tokens are in the StakeAtRisk smart contract.

Tools Used

Manual

Recommended Mitigation Steps

Consider not allowing the user to participate in a battle when amountStaked = 0:

function updateBattleRecord(
        uint256 tokenId,
        uint256 mergingPortion,
        uint8 battleResult,
        uint256 eloFactor,
        bool initiatorBool
    ) external {
        // code
        uint256 stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);
-        if (amountStaked[tokenId] + stakeAtRisk > 0) {
+        if (amountStaked[tokenId] != 0 && amountStaked[tokenId] + stakeAtRisk > 0) {
            _addResultPoints(
                battleResult,
                tokenId,
                eloFactor,
                mergingPortion,
                fighterOwner
            );
        }
        // code
    }

Assessed type

Invalid Validation