AI Arena - 0xabhay's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L454-L455 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L496 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L342-L343

Vulnerability details

Impact

The RankedBattle smart contract poses a significant risk to the integrity of the staking mechanism. A user can exploit this vulnerability to avoid losing any more staked NRN tokens after a portion of their stake is at risk. By Unstaking all staked tokens except the at-risk portion, the user's amountStaked becomes zero. Subsequently, if the user loses a battle, the contract attempts to transfer NRN tokens to the StakeAtRisk address and then subtracts the curStakeAtRisk from the user's amountStaked. Since amountStaked is already zero, this subtraction would cause an underflow, allowing the user to avoid any further losses.

Proof of Concept

A user stakes a significant number of NRN tokens using the RankedBattle.sol::stakeNRN function and participates in battles until a portion of their stake is at risk. The user then calls RankedBattle.sol::unstakeNRN to withdraw all staked tokens except those at risk, effectively setting their amountStaked for the given tokenId to zero. When the user participates in subsequent battles, the updateBattleRecord function is invoked, and the user passes the initial check because their amountStaked[tokenId] is zero, but they have a non-zero stakeAtRisk.

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

Within the updateBattleRecord function, calls the _addResultPoints: Within the _addResultPoints function, the curStakeAtRisk is calculated as follows:

    curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;
    // Given that bpsLostPerLoss is 10 basis points, amountStaked[tokenId] is 0, and stakeAtRisk is 1000 * 1e18, the curStakeAtRisk calculation results in a value greater than the amountStaked[tokenId]. 
    // try out in remix run the following code:
    // SPDX-License-Identifier: MIT
pragma solidity 0.8.22;

    contract PreccisionLoss{

    uint public curStakeAtRiskANS;
    uint public BPS = 10;
    uint public AmountStaked = 0;
    uint public StakeAtrisk = 1000 * 1e18;

    function curStakeAtRisk() public  returns (uint) {

       curStakeAtRiskANS = (BPS * (AmountStaked + StakeAtrisk)) / 10**4;
       return curStakeAtRiskANS;
        }

        //  output = 1000000000000000000
}

If the user loses the battle, the following conditional logic is executed:

// the user have curStakeAmount > amountStaked[tokenId])
if (curStakeAtRisk > amountStaked[tokenId]) {
    curStakeAtRisk = amountStaked[tokenId];
    // curStakeAtRisk is now set to 0 because amountStaked[tokenId] is 0
}
// As a result, curStakeAtRisk is incorrectly set to zero due to the user's amountStaked being zero. This leads to the following problematic behavior:

bool success = _neuronInstance.transfer(_stakeAtRiskAddress, curStakeAtRisk);
if (success) {
    _stakeAtRiskInstance.updateAtRiskRecords(curStakeAtRisk, tokenId, fighterOwner);
    // An underflow occurs here because curStakeAtRisk is non-zero (due to tokens still in stakeAtRisk),
    // but amountStaked[tokenId] is zero. This causes a revert due to underflow, preventing the user from
    // losing any NRN tokens even though they lost the battle. Additionally, the contract fails to update
    // the battle record for the tokenId because of this underflow revert.
    amountStaked[tokenId] -= curStakeAtRisk;
}

The transaction reverts due to the underflow, and the user avoids losing any NRN tokens despite losing the battle. Furthermore, the contract is unable to update the battle record state changes for the tokenId.

The user can exploit this vulnerability by repeating the process: they can unstake any reclaimedTokens, setting amountStaked back to zero, and continue to participate in battles without any additional risk. This cycle can be repeated until the user has fully recovered the entire stakeAtRisk amount, effectively nullifying the intended risk-reward dynamic of the staking system.

Tools Used

Manual Review

Recommended Mitigation Steps

In UpdateBattleRecords:

    function updateBattleRecord(
        uint256 tokenId, 
        uint256 mergingPortion,
        uint8 battleResult,
        uint256 eloFactor,
        bool initiatorBool
    ) 
        external 
    {   
        require(msg.sender == _gameServerAddress);
        require(mergingPortion <= 100);
        address fighterOwner = _fighterFarmInstance.ownerOf(tokenId);
        require(
            !initiatorBool ||
            _voltageManagerInstance.ownerVoltageReplenishTime(fighterOwner) <= block.timestamp || 
            _voltageManagerInstance.ownerVoltage(fighterOwner) >= VOLTAGE_COST
        );

        _updateRecord(tokenId, battleResult);
        uint256 stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);
-        if (amountStaked[tokenId] + stakeAtRisk > 0)
+        if (amountStaked[tokenId] + stakeAtRisk > 0 && amountStaked [tokenId > 0]){
           _addResultPoints(battleResult, tokenId, eloFactor, mergingPortion, fighterOwner);
        }
        if (initiatorBool) {
            _voltageManagerInstance.spendVoltage(fighterOwner, VOLTAGE_COST);
        }
        totalBattles += 1;
    }

2.Introduce a minimum stake requirement for participating in battles to prevent users from gaming the system by staking zero tokens.

Assessed type

Under/Overflow