AI Arena - jesusrod15's results

Lines of code

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/MergingPool.sol#L154 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L313-L331 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L495 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L33

Vulnerability details

Impact

the user could be unable to claimRewads

Proof of Concept

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);
        }
    }

    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);
        uint256 element; 
        uint256 weight;
        uint256 newDna;
        if (customAttributes[0] == 100) {
            (element, weight, newDna) = _createFighterBase(dna, fighterType);
        }
        else {
            element = customAttributes[0];
            weight = customAttributes[1];
            newDna = dna;
        }
        uint256 newId = fighters.length;

        bool dendroidBool = fighterType == 1;
        FighterOps.FighterPhysicalAttributes memory attrs = _aiArenaHelperInstance.createPhysicalAttributes(
            newDna,
            generation[fighterType],
            iconsType,
            dendroidBool
        );
        fighters.push(
            FighterOps.Fighter(
                weight,
                element,
                attrs,
                newId,
                modelHash,
                modelType,
                generation[fighterType],
                iconsType,
                dendroidBool
            )
        );
        _safeMint(to, newId);
        FighterOps.fighterCreatedEmitter(newId, weight, element, generation[fighterType]);
    }

When a user is picked as a winner in a round, they can claim their rewards by calling claimRewards. This function also mints a new fighter from the merging pool inside a for loop that iterates from lowerBound to currentRound. However, the issue is that the for loop can revert all transactions if one mint in the batch reverts.

Specifically, this can revert in the following scenario: Suppose a user has 5 fighters, and they are picked as a winner 5 more times. When the user attempts to call claimRewards, the transaction reverts because of the line

require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);

inside the _createNewFighter function. In this case, MAX_FIGHTERS_ALLOWED is set to 10. Therefore, the user is unable to claim even the 4 remaining fighters they are allowed to claim without exceeding the maximum limit.

POC

1. The user has 5 fighters in their account.
2. The user is picked as a winner 5 times.
3. The user wants to call claimRewards.
4. The transaction reverts because the user attempts to have 10 fighters, exceeding the maximum limit of 9.
5. The user is unable to claim the remaining 4 fighters they are allowed to have.

Tools Used

manual review

Recommended Mitigation Steps

could add a break in the foor loop

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]) {

     +++++++    if(_fighterFarmInstance.balanceOf(to) == _fighterFarmInstance.MAX_FIGHTERS_ALLOWE) 
                    break;

                    _fighterFarmInstance.mintFromMergingPool(
                        msg.sender,                        
                        modelURIs[claimIndex],              
                        modelTypes[claimIndex],             claim other 4
                        customAttributes[claimIndex]
                    );
          ...
                }
            }
        }
   ..
        }
    }

Assessed type

Loop