c4-bot-8 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L519-L534 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L438-L439

Vulnerability details

Impact

Players exploit the staking logic to have a chance to win an NFT from the merging pool with zero risk.

Proof of Concept

Initiating the Exploit:
- Players discover that by staking the smallest possible amount of the game's currency (1 wei of NRN), they can engage in the game's battle mechanics with virtually no financial risk. This is made possible due to a safeguard in the _getStakingFactor function that rounds up the staking factor to 1 for any negligeable stake, ensuring players receive a minimum reward multiplier.
```
function _getStakingFactor(
uint256 tokenId, 
uint256 stakeAtRisk
) 
private 
view 
returns (uint256) 
{
uint256 stakingFactor_ = FixedPointMathLib.sqrt(
  (amountStaked[tokenId] + stakeAtRisk) / 10**18
);
if (stakingFactor_ == 0) {
stakingFactor_ = 1;
}
return stakingFactor_;
} 
```
Allocating Points to Avoid Losses:
- To circumvent the potential loss of points during defeats, players allocate 100% of their battle points to the merging pool. This strategy ensures that even in the event of a loss, the minimal stake does not significantly diminish, as the game's logic rounds down any deductions from such a small stake to zero.
```
/// Potential amount of NRNs to put at risk or retrieve from the stake-at-risk contract
curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;
```
Exploiting Wins for Merging Pool Benefits:
- When players win battles, their allocated points contribute to the merging pool instead of increasing their direct score. This indirect accumulation of benefits allows players to participate in the reward system without risking point loss from future battles. Over time, even with minimal initial investment, players can amass enough points in the merging pool to qualify for NFT rewards, exploiting the system's mechanics.
Scaling the Strategy:
- The exploit's effectiveness is magnified when applied across multiple fighters controlled by the same player. By replicating this minimal stake and full point allocation strategy, a player can significantly increase their chances of winning NFT rewards across their roster of fighters with minimal aggregate investment.
Systemic Impact:
- This strategy, particularly when employed en masse by numerous players, could lead to a substantial imbalance in the game's economic system. The intended risk-reward balance is disrupted, as players find a low-risk loophole that allows for potential rewards, diminishing the value and impact of more significant, engaged player investments.

Coded PoC

Output

Ran 1 test for test/RankedBattle.t - Copy .sol:RankedBattleTest
[PASS] testPlayerGainingPointsWhileNotRiskingStake() (gas: 879241)
Logs:
  Stake at risk after the player loss: 0
  Merging points upon player win: 1500
  Merging points upon player loss: 1500

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

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

Note: The PoC can be ran by adding it to the RankedBattle.t.sol file
forge test --match-test "testPlayerGainingPointsWhileNotRiskingStake" -vv

Code

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

        // 1. Stake a dust amount.
        vm.prank(player);
        _rankedBattleContract.stakeNRN(1, tokenId);
        assertEq(_rankedBattleContract.amountStaked(tokenId) > 0, true);

        // 2. Player will not loss anything due to rounding error.
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(tokenId, 100, 2, 1500, true);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(tokenId), 0);
        console.log("Stake at risk after the player loss: %s",_stakeAtRiskContract.getStakeAtRisk(tokenId));

        // 3. Player gain points upon winning since factor is not 0
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(tokenId, 100, 0, 1500, true);
        uint256 fighterMergingPoints = _mergingPoolContract.getFighterPoints(1)[0];
        assertEq(fighterMergingPoints > 0, true);
        console.log("Merging points upon player win: %s",fighterMergingPoints);

        // 4. Player will not loss points nor stake upon losing.
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(tokenId, 100, 2, 1500, true);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(tokenId), 0);
        assertEq(_mergingPoolContract.getFighterPoints(1)[0], fighterMergingPoints);

        console.log("Merging points upon player loss: %s",fighterMergingPoints);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Revise Rounding Logic: Reassess and adjust the rounding logic within _getStakingFactor by rounding down towards the users. In addition to that, it is recommended to round up the curStakeAtRisk to prevent dust amounts from bypassing the stake loss.

Assessed type

Math

c4-pre-sort commented 7 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #38

c4-judge commented 6 months ago

HickupHH3 changed the severity to 3 (High Risk)