No verification that attributes probabilities exist before creating a fighter

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L169-L187

Vulnerability details

Description

The function AiArenaHelper::dnaToIndex permits returning a number between 1 and 10 to indicate a physical attribute in a generation. However, it never checks if probabilities exist before calculating the index. In the case where AiArenaHelper::deleteAttributeProbabilities is called or a new generation is created for a fighter, there is a timelapse where players can create a fighter with all attributes set to index 0, which is not a possible value according to the protocol. It would lead to unexpected behavior on the game server.

    function dnaToIndex(
        uint256 generation,
        uint256 rarityRank,
        string memory attribute
    ) public view returns (uint256 attributeProbabilityIndex) {
@>        uint8[] memory attrProbabilities = getAttributeProbabilities(
            generation,
            attribute
        );

        uint256 cumProb = 0;
@>        uint256 attrProbabilitiesLength = attrProbabilities.length;
        for (uint8 i = 0; i < attrProbabilitiesLength; i++) {
            cumProb += attrProbabilities[i];
            if (cumProb >= rarityRank) {
                attributeProbabilityIndex = i + 1;
                break;
            }
        }
@>        return attributeProbabilityIndex;
    }

Impact

Likelihood: Medium

• When attributes probabilities are deleted.
• Each new generation of fighters if the attributes probabilities are not be set before.

Impact: Medium/High

• Attacker can frontrun the attributes probabilities setter and create a fighter with 0 value in all attributes. As 0 is an invalid number, it will trigger unexpected behavior on the game server.

Proof of Concept

Foundry PoC to add in Fighter.t.sol.

    function testClaimFightersDuringProbabilitiesDeletion() public {
        uint8[2] memory numToMint = [1, 0];
        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";

        _helperContract.deleteAttributeProbabilities(0);

        // Right sender of signature should be able to claim fighter
        _fighterFarmContract.claimFighters(
            numToMint,
            claimSignature,
            claimModelHashes,
            claimModelTypes
        );
        assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 1);
        assertEq(_fighterFarmContract.ownerOf(0), _ownerAddress);
        assertEq(_fighterFarmContract.totalSupply(), 1);

        (
            uint256 weight,
            uint256 element,
            FighterOps.FighterPhysicalAttributes memory physicalAttributes,
            uint256 id,
            string memory modelHash,
            string memory modelType,
            uint8 generation,
            uint8 iconsType,
            bool dendroidBool
        ) = _fighterFarmContract.fighters(0);

        // All attributes are set to 0 !!
        assertEq(physicalAttributes.head, 0);
        assertEq(physicalAttributes.eyes, 0);
        assertEq(physicalAttributes.mouth, 0);
        assertEq(physicalAttributes.body, 0);
        assertEq(physicalAttributes.hands, 0);
        assertEq(physicalAttributes.feet, 0);
    }

Recommended Mitigation

Add checks before every creation process to be sure that probability arrays are not empty. Alternatively, implement a locker at the end of AiArenaHelper::deleteAttributeProbabilities and unlock fighter creation once new probabilities are set. A simple mitigation to prevent frontrun is also to make the deletion and the setting in the same transaction to prevent any frontrun.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #73

[c4-judge]

HickupHH3 changed the severity to QA (Quality Assurance)