AI Arena - y4y's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/MergingPool.sol#L139

Vulnerability details

Impact

When the roundId is too high and user is a first time reward claimer, the function call can potentially cause DoS due to out-of-gas.

Proof of Concept

In claimRewards, the function is doing a loop for the last round msg.sender has claimed reward, and for each round, another loop with length of round winner length is also applied. When the rounds start to build up and get high, this can potentially cause huge amount of gas usage. To an extreme point, it may even block users' reward claim because of out-of-gas or gas exceeds block gas limit.

        uint256 winnersLength;
        uint32 claimIndex = 0;
        uint32 lowerBound = numRoundsClaimed[msg.sender];
        for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
            numRoundsClaimed[msg.sender] += 1;
            winnersLength = winnerAddresses[currentRound].length;
            for (uint32 j = 0; j < winnersLength; j++) {
                if (msg.sender == winnerAddresses[currentRound][j]) {
                    _fighterFarmInstance.mintFromMergingPool(
                        msg.sender,
                        modelURIs[claimIndex],
                        modelTypes[claimIndex],
                        customAttributes[claimIndex]
                    );
                    claimIndex += 1;
                }
            }
        }

By default, the winner count per round is set to 2 in the contract, and in the test contracts, they are updated to 3. As the round progress, it's more likely for the scenario to happen. Suppose it's current round 20, and Alice is aware that she has some rewards to claim in the merging pool contract, since this is her first time to claim rewards, to it would take around 20 * 2 = 40 total loops to complete this function. 40 loops itself can already cost huge amount of gas, and with time passes, the cost can only increase, unless each player claim rewards after each round has concluded.

Tools Used

Manual review

Recommended Mitigation Steps

Considering adding a map to store winning info for each round, such as:

mapping(uint256 => map(address => bool)) public winnerAddresses

when checking if an address has won anything in the round:

                if (winnerAddresses[currentRound][msg.sender]) {
                    _fighterFarmInstance.mintFromMergingPool(
                        msg.sender,
                        modelURIs[claimIndex],
                        modelTypes[claimIndex],
                        customAttributes[claimIndex]
                    );
                    claimIndex += 1;
                }

Assessed type

DoS