code-423n4 / 2024-02-ai-arena-findings

4 stars 3 forks source link

Lock Persistence Issue for Fighters Losing All Stake in a Round #2034

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

Players face an unintended gameplay interruption as their fighter is kept locked when all their stake goes at risk, with the only resolution being a counterintuitive zero-value unstake action that sidelines them for the remainder of the round.

Proof of Concept

  1. Initial Gameplay Scenario:

    • Players engage their fighters in battles, staking NRN tokens The game's mechanics include the risk of losing staked amounts based on battle performances.
  2. Complete Loss of Stake:

    • During a particularly challenging round, a player’s fighter can lose all its staked NRN tokens, which leaves their fighter locked.
  3. Discovery of Lock Mechanism:

    • The player realizes that the fighter remains locked even after the battle's conclusion, with no automatic mechanism for unlocking at the round's end or the start of a new round. This lock prevents any form transfers on the fighter.
  4. Forced Unstaking for Unlocking:

    • To unlock the fighter, the player must figure out to perform an unstake operation with a value of 0 NRN. This action, while effectively unlocking the fighter, paradoxically disqualifies them from staking again in the current round due to game rules prohibiting staking after unstaking.
  5. Involuntary Downtime:

    • The consequence is a forced waiting period until the next round begins. This downtime is not by player choice but rather an enforced penalty due to the locking mechanism's unintended persistence, significantly detracting from the game's engagement and continuity.
  6. Strategic Implications and Frustration:

    • This situation places players at a strategic disadvantage, unable to leverage their assets effectively and missing out on potential recoveries or strategic plays in the ongoing round. The frustration from this experience could impact player retention and the overall perception of the game's fairness.

Coded Poc

Output

Ran 1 test for test/RankedBattle.t - Copy .sol:RankedBattleTest
[PASS] testFighterRemainsUnlockedAfterLosingAllStake() (gas: 1739133)
Logs:
  Fighter lost all his stake: 0
  Fighter staked amount: 0, fighter locked status: true

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.05ms

Ran 1 test suite in 3.05ms: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Code

function testFighterRemainsUnlockedAfterLosingAllStake() public {
    address player = vm.addr(3);
    _mintFromMergingPool(player);
    uint8 tokenId = 0;
    _fundUserWith4kNeuronByTreasury(player);

    // We set bpsLostPerLoss to 50% so that we can easily simulate a player that lost all his stake.
    _rankedBattleContract.setBpsLostPerLoss(5000);

    // Stake a dust amount.
    vm.prank(player);
    _rankedBattleContract.stakeNRN(4_000 * 10**8, tokenId);
    assertEq(_rankedBattleContract.amountStaked(tokenId) > 0, true);

    // A losing strick that will put all the fighter stake at risk.
    vm.prank(address(_GAME_SERVER_ADDRESS));
    _rankedBattleContract.updateBattleRecord(tokenId, 50, 2, 1500, true);
    vm.prank(address(_GAME_SERVER_ADDRESS));
    _rankedBattleContract.updateBattleRecord(tokenId, 50, 2, 1500, true);

    assertEq(_rankedBattleContract.amountStaked(tokenId), 0);
    assertEq(_stakeAtRiskContract.getStakeAtRisk(tokenId), 4_000 * 10**8);
    console.log("Fighter lost all his stake: %s",_rankedBattleContract.amountStaked(tokenId));

    // Add a second player with a win record so that totalAccumulatedPoints[roundId] > 0 and set a new round.
    address player2 = vm.addr(4);
    _mintFromMergingPool(player2);
    uint8 secondTokenId = 1;
    _fundUserWith4kNeuronByTreasury(player2);
    vm.prank(player2);
    _rankedBattleContract.stakeNRN(1_000 * 10 ** 18, secondTokenId);
    vm.prank(address(_GAME_SERVER_ADDRESS));
    _rankedBattleContract.updateBattleRecord(secondTokenId, 50, 0, 1500, true);
    _rankedBattleContract.setNewRound();

    // Player try to transfer his fighter
    vm.prank(player);
    uint256 totalStakedAmount = _rankedBattleContract.amountStaked(tokenId) + _stakeAtRiskContract.getStakeAtRisk(tokenId);
    assertEq(totalStakedAmount, 0);
    vm.expectRevert();
    _fighterFarmContract.transferFrom(player, player2, tokenId);
    console.log("Fighter staked amount: %s, fighter locked status: %s",totalStakedAmount,_fighterFarmContract.fighterStaked(tokenId));

    // The player will be forced to call the unstake function with an amount of 0 in order to unlock his fighter, and this will mark the unstaked in round to zero which will prevent the fighter reciepient from participating in the current round.
    vm.prank(player);
    _rankedBattleContract.unstakeNRN(0, tokenId);
    vm.prank(player);
    _fighterFarmContract.transferFrom(player, player2, tokenId);
    assertEq(_rankedBattleContract.hasUnstaked(tokenId, 1), true);
}

Tools Used

Manual review.

Recommended Mitigation Steps

Consider unlocking the fighter whenever the staker loses all the stake at risk.

Assessed type

Other

c4-pre-sort commented 8 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #1641

c4-pre-sort commented 8 months ago

raymondfam marked the issue as not a duplicate

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #833

raymondfam commented 8 months ago

Just keep playing before the round ends trying to win and reclaim some stakeatrisk NRN risk free.

c4-judge commented 8 months ago

HickupHH3 marked the issue as duplicate of #1641

c4-judge commented 8 months ago

HickupHH3 marked the issue as partial-25