AI Arena - inzinko's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/MergingPool.sol#L118-L132 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/MergingPool.sol#L139-L167 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L484-L495

Vulnerability details

Impact

If a user gets Selected as winner in a round in the merging pool contract, they are added to a winnersAddress mapping, and they can claim their reward in the form of a fighter which has to be minted in the FighterFarm contract, This vunerability comes up in a specific situation, where a user that does not claim their fighter after every round and it accumulates to up to 11 rounds where they are entitled to receive a NFT, then when the user is about to call claimRewards in the Merging Pool contract, and the minting is called, this action will fail due to the require(balanceOf(to) < MAX_FIGHTERS_ALLOWED); check in the code. This will lead to a situation where claimRewards will be DOS and the affected user will be unable to claim rewards again, even if they are added as winners in the future.

Proof of Concept

Users are added as winners for a particular round from the admins through the pickWinner function

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

Inside that function the currentWinnerAddresses array is updated IN accordance with the winners per period requirement as shown below

...more code
for (uint256 i = 0; i < winnersLength; i++) {
            currentWinnerAddresses[i] = _fighterFarmInstance.ownerOf(winners[i]);
            totalPoints -= fighterPoints[winners[i]];
            fighterPoints[winners[i]] = 0;
        }
        winnerAddresses[roundId] = currentWinnerAddresses;
        isSelectionComplete[roundId] = true;
        roundId += 1;

When a user wants to claim their rewards they call the claimRewards function that enables them as shown below

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

function claimRewards(
        string[] calldata modelURIs, 
        string[] calldata modelTypes,
        uint256[2][] calldata customAttributes
    ) 
        external 
    {
        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;
                }
            }
        }
        if (claimIndex > 0) {
            emit Claimed(msg.sender, claimIndex);
        }
    }

The issue as described in the impact section refers to a peculiar situation where a user or group of users, are not consistent claimers, which means they let their rewards accumulate, if now, a particular user is added to the winners array 11 diffrent times and they refuse to claim their rewards, any call to claimRewards function will lead to a revert as the check require(balanceOf(to) < MAX_FIGHTERS_ALLOWED); in the _createNewFighter function in the FighterFarm contract will revert when the check gets to the 11th fighter to mint that check will fail and the whole operation will revert, leading to a DOS. This check is shown below

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L484C4-L495C55

 function _createNewFighter(
        address to, 
        uint256 dna, 
        string memory modelHash,
        string memory modelType, 
        uint8 fighterType,
        uint8 iconsType,
        uint256[2] memory customAttributes
    ) 
        private 
    {  
        require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);
...more code

As Clearification for the Particular Number 11 that will result in DOS, This number is gotten when you think about it like this

• If the number of Uncliamed NFT was 10, the affected user may simply if they wish transfer the NFTs they have currently just to get around the minting check
• But by having 11+ unclaimed, The check cant be passed Even if a transfer of current NFTs occurs, Hereby resulting in DOS

Tools Used

Maual Reveiew

Recommended Mitigation Steps

A maximum Number of Pending or Unclaimed Rewards Limit should be set, if a User has a Certain number of Unclaimed Rewards That User Should be Unable to be added as a winner that would be entitled to receive Fighters, This Would prevent situations described in the Report.

Assessed type

DoS