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

4 stars 3 forks source link

If AI Arena amasses millions of players, the protocol can reach a state of permanent DoS due to storing the aggregate number of training sessions in a uint32 #62

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L45 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L294

Vulnerability details

Impact

If AI Arena amasses millions of active players, the protocol can reach a permanent state of DoS due to storing all training sessions in a uint32.

Each time a training session has been conducted for a fighter it is recorded in the totalNumTrained variable which represents the total number of sessions conducted on a protocol level.

It is encouraged by the protocol for the fighters to undergo frequent training in order for them to "learn", "adapt" and fight better in the arena. Since every training session/model update or whenever FighterFarm::updateModel() is called for a specific fighter, it is recorded in the totalNumTrained variable. The maximum number which can be stored in totalNumTrained is 4.3 billion, and if the protocol reaches this number, no more model updates can be performed due to updateModel() reverting because of overflow. This will cause a permanent DoS and the models would never be able to be updated or when a new fighter is created for it to be trained.

Proof of Concept

The maximum number which can be stored in a uint32 is 4,294,967,295 (almost 4.3 billion). Although this seems like a large number, it is plausible to reach it if the protocol has millions of active players over a longer period of time.

Whenever updateModel() is called, totalNumTrained() is updated each time:

    function updateModel(
        uint256 tokenId, 
        string calldata modelHash,
        string calldata modelType
    ) 
        external
    {
        require(msg.sender == ownerOf(tokenId));
        fighters[tokenId].modelHash = modelHash;
        fighters[tokenId].modelType = modelType;
        numTrained[tokenId] += 1;
        totalNumTrained += 1;
    }
    /// @notice Aggregate number of training sessions recorded.
    uint32 public totalNumTrained;

Let's take a look at the active player figures of some of the more popular blockchain games:

And the above figures can rapidly increase during bull markets or as the blockchain/crypto adoption increases.

A simplified hypothetical scenario is presented below to serve as a PoC.

The above scenario shows that the protocol will reach a permanent state of DoS within 4 years (or sooner if the numbers increase). Once the maximum amount of sessions are reached updateModel() will panic revert every time that it is called due to overflow.

This would mean that existing fighters can't be trained anymore and that every new fighter which is created will be unusable since you can't assign a model to it / train it.

Tools Used

Manual Review

Recommended Mitigation Steps

Don't store the aggregate number of training sessions totalNumTrained in a uint32, make it a uint128 or uint256.

Assessed type

DoS

c4-pre-sort commented 9 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 9 months ago

raymondfam marked the issue as primary issue

raymondfam commented 9 months ago

Informative but QA at best.

HickupHH3 commented 8 months ago

QA(R)

c4-judge commented 8 months ago

HickupHH3 changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

HickupHH3 marked the issue as grade-c