code-423n4 / 2024-02-ai-arena-findings

4 stars 3 forks source link

FighterFarm:: reroll won't work for nft id greator than 255 due to input limited to uint8 #68

Open c4-bot-10 opened 9 months ago

c4-bot-10 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

FighterFarm:: reRoll uses uint8 for nft id as input, which will stop people calling this function who owns id greater than 255.It will lead to not being able to use the reRoll to get random traits, which could have been better for there game performance.

Proof of Concept

Affect code can be seen here

Adding code snippet below as well, for better clarity

    /// @notice Rolls a new fighter with random traits.
    /// @param tokenId ID of the fighter being re-rolled.
    /// @param fighterType The fighter type.
@>    function reRoll(uint8 tokenId, uint8 fighterType) public {
        require(msg.sender == ownerOf(tokenId));
        require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]);
        require(_neuronInstance.balanceOf(msg.sender) >= rerollCost, "Not enough NRN for reroll");

        _neuronInstance.approveSpender(msg.sender, rerollCost);
        bool success = _neuronInstance.transferFrom(msg.sender, treasuryAddress, rerollCost);
        if (success) {
            numRerolls[tokenId] += 1;
            uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));
            (uint256 element, uint256 weight, uint256 newDna) = _createFighterBase(dna, fighterType);
            fighters[tokenId].element = element;
            fighters[tokenId].weight = weight;
            fighters[tokenId].physicalAttributes = _aiArenaHelperInstance.createPhysicalAttributes(
                newDna,
                generation[fighterType],
                fighters[tokenId].iconsType,
                fighters[tokenId].dendroidBool
            );
            _tokenURIs[tokenId] = "";
        }
    }   

If you notice the highlighted line (first line of function), it takes uint8 as input for tokenId parameter. Which will restrict users to call this function when they own nft id greater than 255.

value will go out of bounds when user will input 256 or more.

Tools Used

Manual Review

Recommended Mitigation Steps

use uint256 for nft id input to fix the issue.

- function reRoll(uint8 tokenId, uint8 fighterType) public {
+ function reRoll(uint256 tokenId, uint8 fighterType) public {
   require(msg.sender == ownerOf(tokenId));
        require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]);
        require(_neuronInstance.balanceOf(msg.sender) >= rerollCost, "Not enough NRN for reroll");

        _neuronInstance.approveSpender(msg.sender, rerollCost);
        bool success = _neuronInstance.transferFrom(msg.sender, treasuryAddress, rerollCost);
        if (success) {
            numRerolls[tokenId] += 1;
            uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));
            (uint256 element, uint256 weight, uint256 newDna) = _createFighterBase(dna, fighterType);
            fighters[tokenId].element = element;
            fighters[tokenId].weight = weight;
            fighters[tokenId].physicalAttributes = _aiArenaHelperInstance.createPhysicalAttributes(
                newDna,
                generation[fighterType],
                fighters[tokenId].iconsType,
                fighters[tokenId].dendroidBool
            );
            _tokenURIs[tokenId] = "";
        }
    }

Assessed type

DoS

c4-pre-sort commented 8 months ago

raymondfam marked the issue as primary issue

c4-pre-sort commented 8 months ago

raymondfam marked the issue as sufficient quality report

raymondfam commented 8 months ago

Unsigned integer type limitation indeed.

c4-sponsor commented 8 months ago

brandinho (sponsor) confirmed

SonnyCastro commented 8 months ago

Mitigated here

c4-judge commented 8 months ago

HickupHH3 marked the issue as satisfactory

c4-judge commented 8 months ago

HickupHH3 marked the issue as selected for report