It is not possible to create a new figther neither update it on new generations

[c4-bot-2]

Lines of code

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

Vulnerability details

Impact

No new fighters (both Champion and Dendroid fighter types) can be created after generation 0 due to a "Division or modulo by 0" revert on the FighterFarm._createFighterBase function. This private function is externally called by:

• FighterFarm.reRoll.
• FighterFarm.claimFighters.
• FighterFarm.redeemMintPass.
• MergingPool.claimRewards.

Description

In the FighterFarm contract:

1. The numElements mapping maps a generation to a number of elements:

/// @notice Mapping of number elements by generation.
mapping(uint8 => uint8) public numElements;

2. The constructor sets the elements of the generation 0 in numElements:

/// @notice Sets the owner address, the delegated address.
/// @param ownerAddress Address of contract deployer.
/// @param delegatedAddress Address of delegated signer for messages.
/// @param treasuryAddress_ Community treasury address.
constructor(address ownerAddress, address delegatedAddress, address treasuryAddress_)
    ERC721("AI Arena Fighter", "FTR")
{
    _ownerAddress = ownerAddress;
    _delegatedAddress = delegatedAddress;
    treasuryAddress = treasuryAddress_;
    numElements[0] = 3; // @audit here
}

3. The contract has no numElements setter neither an assignment expression that sets numElements on future generations.

4. The _createFighterBase function calculates element of the given fighter type by computing the the modulo operation mod(dna, x), being x the numElements of the current generation for the given fighter type:

uint256 element = dna % numElements[generation[fighterType]];

5. This computation will revert due to a "Division or modulo by 0" for any generation greater than 0.

Proof Of Concept

Add the following tests in FighterFarm.t.sol:

function testClaimFightersRevertForChampionWithGenerationGtZero() public {
    // Arrange
    uint8[2] memory numToMint = [1, 0]; // NB: fighter type is Champion
    bytes memory claimSignature = abi.encodePacked(
        hex"407c44926b6805cf9755a88022102a9cb21cde80a210bc3ad1db2880f6ea16fa4e1363e7817d5d87e4e64ba29d59aedfb64524620e2180f41ff82ca9edf942d01c"
    );
    string[] memory claimModelHashes = new string[](1);
    claimModelHashes[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

    string[] memory claimModelTypes = new string[](1);
    claimModelTypes[0] = "original";

    // NB: increment Champion generation from 0 to 1
    _fighterFarmContract.incrementGeneration(0);
    assertEq(_fighterFarmContract.generation(0), 1);

    // Act
    vm.expectRevert();
    _fighterFarmContract.claimFighters(numToMint, claimSignature, claimModelHashes, claimModelTypes);

    // Assert
    assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 0);
    assertEq(_fighterFarmContract.totalSupply(), 0);
}

function testClaimFightersRevertForDendroidWithGenerationGtZero() public {
    // Arrange
    uint8[2] memory numToMint = [0, 1]; // NB: fighter type is Dendroid
    bytes memory claimSignature = abi.encodePacked(
        hex"407c44926b6805cf9755a88022102a9cb21cde80a210bc3ad1db2880f6ea16fa4e1363e7817d5d87e4e64ba29d59aedfb64524620e2180f41ff82ca9edf942d01c"
    );
    string[] memory claimModelHashes = new string[](1);
    claimModelHashes[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

    string[] memory claimModelTypes = new string[](1);
    claimModelTypes[0] = "original";

    // NB: increment Dendroid generation from 0 to 1
    _fighterFarmContract.incrementGeneration(1);
    assertEq(_fighterFarmContract.generation(1), 1);

    // Act
    vm.expectRevert();
    _fighterFarmContract.claimFighters(numToMint, claimSignature, claimModelHashes, claimModelTypes);

    // Assert
    assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 0);
    assertEq(_fighterFarmContract.totalSupply(), 0);
}

Tools Used

Forge tests and manually reviewed.

Recommended Mitigation Steps

Add a numElements setter (with the access controlled) and make sure to properly set it for the upcoming generations. Also consider to do not allow increment generation (via the incrementGeneration function) if numElements for the upcoming generation have not been set yet.

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #45

[c4-judge]

HickupHH3 marked the issue as satisfactory