`maxRerollsAllowed` can be breached by the fighter's owner`..,

[c4-bot-3]

Lines of code

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

Vulnerability details

Impact

This impacts is not severe, but medium due to the invarianting breaking, to allow only max rerlls per that fighter. And it can only rerolled if new generation is increased.

Code from FighterFarm::reRoll

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

       ------------------- skimmed ------------------------
    }    

But the token owner can call reroll with fighterType as param, so for normal bot (not dendroid) the owner will reroll max allowed for that fighter type. But after reaching maximum, he can again call reroll with different fighterType == 1 (dendroid), so now if the max rerolls for dendroid is greater than normal bots, then it will be rerolled., so now technically max rerolls are breached, and new skin/weights/looks for that tokens are obtained.

Proof of Concept

As you can see the logs, the max rerolls is breached by the token id 0's owner.

Running 1 test for test/FighterFarm.t.sol:FighterFarmTest
[PASS] test_POC_breach_maxRerolls() (gas: 748831)
Logs:
  Before numRerolls(0):  0
  After numRerolls(0):  4
  maxRerollsAllowed(0):  3

For thr POC to run, paste the below code into FighterFarm:: to make rerolls possible, or else it will revert due to modulo by zero error. Because new elements implementation is missed, and it is covered in a separate issue. This issue is about breaching max rerolls, so even after that modulo by zero issue been fixed, this issue will remain beacuse the root causes are different

    function setElements(uint8 gen, uint8 elements) external {
        require(msg.sender == _ownerAddress);
        numElements[gen] = elements;
    }

-Now paste the below test/FighterFarm.t.sol and run forge t --mt test_POC_breach_maxRerolls -vvvv

    function test_POC_breach_maxRerolls() public {

        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _neuronContract.addSpender(address(_fighterFarmContract));

        // mint
        _mintFromMergingPool(_ownerAddress);
        assertEq(_fighterFarmContract.ownerOf(0), _ownerAddress);

        // incrementGeneration and elements
        uint8 gen = 1;
        assertEq(_fighterFarmContract.generation(gen), 0);
        _fighterFarmContract.incrementGeneration(gen); // incrementing fighter type 1
        assertEq(_fighterFarmContract.generation(gen), 1);
        _fighterFarmContract.setElements(gen, 3); // or else it will revert, not possible to reroll due to modulo by 0 revert panic

        console.log('Before numRerolls(0): ', _fighterFarmContract.numRerolls(0));

        // rerolling now
        uint8 fighterType = 0;

        _fighterFarmContract.reRoll(0, fighterType);
        _fighterFarmContract.reRoll(0, fighterType);
        _fighterFarmContract.reRoll(0, fighterType);

        fighterType = 1;
        _fighterFarmContract.reRoll(0, fighterType);

        console.log('After numRerolls(0): ', _fighterFarmContract.numRerolls(0));
        console.log('maxRerollsAllowed(0): ', _fighterFarmContract.maxRerollsAllowed(0));

    }

Tools Used

Manual + Foundry testing

Recommended Mitigation Steps

Validate FighterFarm::reroll's input param fighterType, if that fighter is a dendroid or a normal bot.

    function reRoll(uint8 tokenId, uint8 fighterType) public {
        require(msg.sender == ownerOf(tokenId));
+       if (fighterType == 0) require(!fighters[tokenId].dendroidBool);
+       else require(fighters[tokenId].dendroidBool);
        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

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #306

[c4-judge]

HickupHH3 marked the issue as satisfactory

[c4-judge]

HickupHH3 changed the severity to 3 (High Risk)