Submitted by ahmedaghadi, also found by SpicyMeatball (1, 2), btk, 0xKowalski, ktg (1, 2), 0xDetermination (1, 2), Ryonen, sl1 (1, 2), MrPotatoMagic, klau5 (1, 2, 3), vnavascues, pontifex, Nyxaris, alexzoid, inzinko, grearlake (1, 2), Honour, cu5t0mpeo, evmboi32, Avci, Greed, josephdara, VAD37, DeFiHackLabs, 0xPluto, radev_sw (1, 2), MidgarAudits, KmanOfficial, soliditywala, ladboy233, AlexCzm, jesusrod15, erosjohn, merlinboii, taner2344, Krace, ke1caM (1, 2), BigVeezus, Draiakoo, peanuts (1, 2), McToady (1, 2), Giorgio, 0x13, deadrxsezzz, SovaSlava, almurhasan (1, 2), 0xvj, _eperezok (1, 2), emrekocak, fnanni, pipidu83, y4y, jesjupyter, BARW (1, 2), djxploit, sobieski, ReadyPlayer2 (1, 2), Fitro, 0xAleko, zaevlad, GoSlang, 0xRiO, Cryptor, Kalogerone, t0x1c, nuthan2x, yovchev_yoan, and dvrkzy
The MergingPool::claimRewards function loop through last claimed round ID to the latest round ID ( https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/MergingPool.sol#L139 ):
Also theres another nested loop which loops through all the winners each round. Thus, it will become very expensive to claim rewards and eventually leads to block gas limit. Due to which some users may never be able to claim their rewards.
Therefore, If user try to claim their rewards after many rounds has passed then due to the above mentioned loops, it will consume a lot of gas and eventually leads to block gas limit.
Similarly, the RankedBattle::claimNRN function loop through last claimed round ID to the latest round ID ( https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L294 ) :
Although, its relatively difficult to reach the block gas limit in claimNRN function as compared to claimRewards function, but still its possible.
For claimRewards function, Add the below function in MergingPool.t.sol:
You can run the test by:
Here I had considered 3 users, user1, user2, and user3. After each offset rounds, I picked user3 as a winner. There were total of 315 ( offset * totalWin ) rounds passed and user3 won in 9 of them. Then I tried to claim rewards for user3 and it consumed more than 4M gas.
Also the more the round in which user3 has won, the more gas it will consume. Even if offset is 1 and totalWin is 9 ( i.e total of 9 rounds out of which user3 won in 9 of them ), it will consume more than 3.4M gas.
Also, weve considered only 2 winners per round, as the number of winners increases, the gas consumption will also increase due to the nested loop which loops through all the winners each round.
So if any user claim their rewards after many rounds has passed or if they have won in many rounds, it will consume a lot of gas and eventually leads to block gas limit.
For claimNRN function, Add the below function in RankedBattle.t.sol:
You can run the test by:
In the case of claimNRN function, it consumed more than 1M gas which is relatively less as compared to claimRewards function. But still it has potential to reach block gas limit.
Even for the users for whom these both functions doesnt reach block gas limit, it can be very expensive and difficult for them to claim their rewards if some rounds has passed.
Foundry
It can be fixed by adding a parameter for the number of rounds to consider.
For claimRewards function, so the changes would look like:
For claimNRN function, so the changes would look like:
brandinho (AI Arena) confirmed
AI Arena mitigated:
Status: Mitigation confirmed. Full details in reports from niser93, fnanni, and d3e4.
