AI Arena - Topmark's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L470

Vulnerability details

Impact

Denial of Service when Reroll(...) is called at L380 and _createNewFighter(...) is called at L500 with Generation of FighterType that is higher than Zero

Proof of Concept

 function _createFighterBase(
        uint256 dna, 
        uint8 fighterType
    ) 
        private 
        view 
        returns (uint256, uint256, uint256) 
    {
>>>        uint256 element = dna % numElements[generation[fighterType]];
        uint256 weight = dna % 31 + 65;
        uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);
        return (element, weight, newDna);
    }

The function above shows how _createFighterBase(...) function is implemented in the FighterFarm.sol contract, as noted from the pointer element is assigned a value by deriving the reminder between the devision of dna and numElements[...], but a look at the contract shows that numElements[...] was only assigned once in the contract at L110 only zero mapping was given a value however it can be noted from the implementation in the contract that generation[fighterType] will not always be zero as can be seen at L131. The implication of this is that when numElements[generation[fighterType]] is called in the _createFighterBase(...) above, it would give zero since numElements of higher generation as not been assigned a value, this will revert the _createFighterBase(...) function as trying the % operator with a value of zero would cause a panic error and revert the function.

Tools Used

Manual Review

Recommended Mitigation Steps

Ai-Arena Protocol should add an implementation that ensures increase in Generation value gives its numElements mapping an actual value just like it was done at L110 of the contract when the Generation value was still Zero. This implementation can be done when incrementGeneration(...) function is called in the contract.

  function incrementGeneration(uint8 fighterType) external returns (uint8) {
        require(msg.sender == _ownerAddress);
        generation[fighterType] += 1;
        maxRerollsAllowed[fighterType] += 1;
+++     /// implementation to assign a value to numElements[generation[fighterType]] here
        return generation[fighterType];
    }

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L499

Vulnerability details

Impact

New Fighter base can be Created Wrongly in the FighterFarm.sol contract there by breaking Protocol

Proof of Concept

The call of _createNewFighter(...) functiion in the FighterFarm contract at L212-L219 & L252-L259 shows that the last parameter i.e customAttributes was called with [uint256(100), uint256(100)], however a look at the implementation of _createNewFighter(...) function as provided in code below from the pointer shows that a check was done to see if customAttributes[0] alone is equal 100, this implementation is incomplete as it is assuming that customAttributes[1] will also always be equal to 100 when customAttributes[0] is equal to 100, but a further look at the contract at L322-L329 shows that having diverse value of customAttributes array is a possibility and a situation where customAttributes[0] is 100 and customAttributes[1] is not 100 is a possibility.

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

Tools Used

Manual Review

Recommended Mitigation Steps

Ai-arena should adjust the _createNewFighter(...) function such that validation is done for both customAttributes[0] & customAttributes[1] to ensure they are both equivalent to 100 and not just one of them as adjusted below before a new Fighter base is created

 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) {
+++        if (customAttributes[0] == 100 && customAttributes[1] == 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]);
    }

Assessed type

Error