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

4 stars 3 forks source link

Fighter rare attributes can be deterministically rerolled #2031

Closed c4-bot-5 closed 8 months ago

c4-bot-5 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol?plain=1#L379

Vulnerability details

Impact

Users can arbitrarily control the rarityRank value, a value designed to determine what attribute a fighter is to retrieve, including the fighter receives a rare attribute. Since the user can control the rarityRank via dna, a user can call FighterFarm.reRoll() to re-roll their physical attributes to receive rare attributes.

Proof of Concept

To begin, DNA from re-rolls are calculated via the fighter owner's address. A fighter owner can transfer the fighter to any arbitrary address they control. Below we can see how the msg.sender acts as an input to generate a dna:

uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));

Next, we'll consider that in order to determine the rarity of attributes, AI Arena utilizes the following formula in the AiArenaHelper contract:

function createPhysicalAttributes(
    ...
    // AUDIT: the rarityRank is calculated based on the modulus of dna. Since dna is a user-controlled value, the hacker also controls the value of the rarityRank. Note that rarityRank is a poor name here as it should be randomNumber.
    uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
    uint256 attributeIndex = dnaToIndex(generation, rarityRank, attributes[i]);
    ...
}

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];
        // The rarityRank is used to generate attribute probabilities. By controlling the rarityRank, the fighter owner can also control the value of attributeProbabilityIndex
        if (cumProb >= rarityRank) {
            attributeProbabilityIndex = i + 1;
            break;
        }
    }
    return attributeProbabilityIndex;
}

As you can see above, the dna's modulus is used to calculate what's called the rarityRank, a pseudorandom number. Although as mentioned previously, it is not a pseudo random number as the user can control the dna value via the fighter owner address.

In anticipation of a re-roll, the fighter owner can generate a series of random addresses via Vanity ETH and determine off-chain which address will give them the best dna to receive rare attributes.

This can result in the following scenario:

  1. Owner receives fighter.
  2. Owner offchain generates an address that gives them a dna that best gives them the best attribute probability indexes.
  3. Owner transfers fighter to generated address.
  4. Owner sends NRN to the generated address.
  5. Owner from the generated address calls FighterFarm.reRoll() which gives the user pre-determined rare attributes based on the generated address.

Tools Used

Manual inspection.

The protocol should not rely on the msg.sender as that value is user-controlled. Instead the protocol should rely on a different pseudorandom number such as block.timestamp and block.difficulty. Although these value could be manipulated by miners, the difficulty is quite high to manipulate such values.

Code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol?plain=1#L379

Assessed type

en/de-code

c4-pre-sort commented 8 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #53

c4-judge commented 8 months ago

HickupHH3 changed the severity to 3 (High Risk)

c4-judge commented 8 months ago

HickupHH3 marked the issue as satisfactory

c4-judge commented 8 months ago

HickupHH3 changed the severity to 2 (Med Risk)

c4-judge commented 8 months ago

HickupHH3 marked the issue as duplicate of #376