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

4 stars 3 forks source link

User can easily trick game server not update their battle record while report opponent lost result in `RankedBattle.sol` by depleted their own energy. Resulted in unfair battle report and rewards distribution. #1572

Closed c4-bot-5 closed 7 months ago

c4-bot-5 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L322-L349

Vulnerability details

For offchain bot/game server, RankedBattle.updateBattleRecord() need to called twice for each battle. One for initiator and one for opponent.

While bot code can check if user have enough energy to battle before simulate and sending battle result. User can manually empty their own energy voltage after battle/result started. Causing updateBattleRecord() transaction for initiator to fail. Resulted in, battle report for initiator never updated but opponent still get their record updated.

This exploit is more beneficial for user with top ranking and lots of stake. Allowing them to never winning/losing points but can decrease/increase other user points rankings. Causing ranking MMR and reward distribution issue.

Impact

It is easy to fail one of two updateBattleRecord() transaction required to update single battle result between 2 players. Unfair battle reports leads to unfair rewards distribution.

Proof of Concept

It is unclear from the game context as when battle result is sent. After battle simulation ended on game server or after user game client finished result simulation. I will just assume in both case, game server only check energy voltage once when game client request a battle.

When battle simulation start on both gameserver and game client, no battle result is sent yet. User depleted their own energy voltage to <10 energy.

When game server finished battle simulation and sending battle result to contract. 2 transactions will be sent to update battle result. One for initiator and one for opponent. One transaction will fail due to not enough energy.

While initiator battle result never updated, opponent battle result is updated. If opponent have any stake, they will lose their points and stake. Causing opponent to lose money and ranking despite no battle taking place. This leads to unfair distribution of rewards for high ranking.

Tools Used

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L322-L349

Recommended Mitigation Steps

It make more sense to update battle result of both initiator and opponent in single transaction. There is no reason for game server to sending 2 transaction to update single battle result. Also split energy voltage check and send battle report into 2 different function and transactions would be better.

The perfect order execution for game server is:

Assessed type

Context

c4-pre-sort commented 7 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #49

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #1846

c4-judge commented 7 months ago

HickupHH3 marked the issue as unsatisfactory: Invalid