AI Arena - d3e4's results

Lines of code

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

Vulnerability details

Impact

safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) is not overridden to check _ableToTransfer() in FighterFarm transfers. Therefore it is possible to hold more than MAX_FIGHTERS_ALLOWED fighters and transfer staked fighters.

This may enable the user to initiate more than 10 battles per day for a fighter, and may DoS updateBattleRecord().

Proof of Concept

FighterFarm inherits from ERC721 which contains three transfer functions. Two of these are overridden to include a require(_ableToTransfer(tokenId, to));

function _ableToTransfer(uint256 tokenId, address to) private view returns(bool) {
    return (
        _isApprovedOrOwner(msg.sender, tokenId) &&
        balanceOf(to) < MAX_FIGHTERS_ALLOWED &&
        !fighterStaked[tokenId]
    );
}

prevents holding more than MAX_FIGHTERS_ALLOWED fighters and prevents transfers of staked fighters. But the public safeTransferFrom() which includes the data parameter is not overridden and therefore void of these new restrictions.

It is therefore possible to transfer fighters such that the recipient holds more than MAX_FIGHTERS_ALLOWED fighters, and to transfer staked fighters.

A consequence of this is that the new owner has a fresh voltage and battles can therefore be initiated without limit.

Since points are added to or deducted from accumulatedPointsPerFighter and accumulatedPointsPerAddress in parallel at every win or loss, transferring the fighter to a new address, for which then accumulatedPointsPerAddress is 0 (or just different) _addResultPoints() will attempt to remove more points then possible, since the points are calculated based on the tokenId rather than the fighterOwner. This will then cause a DoS of updateBattleRecord() due to underflow.

Recommended Mitigation Steps

Override _transfer(), _beforeTokenTransfer() or _afterTokenTransfer() instead.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

User can freely set the DNA when redeeming mint passes such that the user has complete control over the choice of element, weight and physicalAttributes of the minted fighter.

(Note that also iconsTypes can be set freely by the user, which is probably by design, as well as fighterTypes, which on the other hand does seem to go against the stated intention to make dendroids very rare and more difficult to obtain.)

Proof of Concept

FighterFarm.redeemMintPass() simply takes a string[] calldata mintPassDnas each element of which is then hashed as uint256(keccak256(abi.encode(mintPassDnas[i]))) and passed as the dna to each fighter created via _createNewFighter().

In _createNewFighter() this dna is used to set (element, weight, newDna) = _createFighterBase(dna, fighterType) which is calculated as

uint256 element = dna % numElements[generation[fighterType]];
uint256 weight = dna % 31 + 65;
uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);

numElements is by default 3. This is then used to set the FighterPhysicalAttributes via AiArenaHelper.createPhysicalAttributes(). If fighterType == 0 and so dendroidBool == true, this always returns the same result, otherwise the DNA may have an influence in the following way:

uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
uint256 attributeIndex = dnaToIndex(generation, rarityRank, attributes[i]);
finalAttributeProbabilityIndexes[i] = attributeIndex;

which is iterated 6 times. attributeToDnaDivisor are by default [2, 3, 5, 7, 11, 13] so over the 6 attributes the 6 rarityRank takes on at most (actually less) 2 * 3 * 5 * 7 * 11 * 13 * 100 = 3003000 different combinations. Counting also the element and weight combinations we only need to brute force 3 * 31 * 3003000 = 279_279_000 different DNAs to get precisely the desired fighter. Thus can the user completely bypass the pseudo-randomness imposed by the DNA mechanism.

Recommended Mitigation Steps

Set the DNA in some other manner, ideally using Chainlink VRF, or by block.prevrandao, or by incorporating the DNA in the mint passes themselves and have the signer of these set them.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

By letting the user choose fighterType for the token he is about to reroll, the user can influence the number of rerolls he can attempt, the fighter's new element, weight and physicalAttributes.

Proof of Concept

FighterFarm.reRoll() lets the user choose the fighterType.

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);
    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] = "";
    }
}

fighterType is used in three places. The user can select fighterType to maximize maxRerollsAllowed. In _createFighterBase() the choice of fighterType has the following effect.

uint256 element = dna % numElements[generation[fighterType]];
uint256 weight = dna % 31 + 65;
uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);
return (element, weight, newDna);

That is, it may be chosen to get better attributes. In createPhysicalAttributes() the choice of generation[fighterType] has the following effect.

if (dendroidBool) {
    return FighterOps.FighterPhysicalAttributes(99, 99, 99, 99, 99, 99);
} else {
    uint256[] memory finalAttributeProbabilityIndexes = new uint[](attributes.length);

    uint256 attributesLength = attributes.length;
    for (uint8 i = 0; i < attributesLength; i++) {
        if (
            i == 0 && iconsType == 2 || // Custom icons head (beta helmet)
            i == 1 && iconsType > 0 || // Custom icons eyes (red diamond)
            i == 4 && iconsType == 3 // Custom icons hands (bowling ball)
        ) {
            finalAttributeProbabilityIndexes[i] = 50;
        } else {
            uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
            uint256 attributeIndex = dnaToIndex(generation, rarityRank, attributes[i]);
            finalAttributeProbabilityIndexes[i] = attributeIndex;
        }
    }
    return FighterOps.FighterPhysicalAttributes(
        finalAttributeProbabilityIndexes[0],
        finalAttributeProbabilityIndexes[1],
        finalAttributeProbabilityIndexes[2],
        finalAttributeProbabilityIndexes[3],
        finalAttributeProbabilityIndexes[4],
        finalAttributeProbabilityIndexes[5]
    );
}

If dendroidBool == false (in which case fighterType = 0 and newDna == dna) the choice of generation determines the attribute probabilities used to determine the attributeIndex.

Recommended Mitigation Steps

Since dendroidBool = fighterType == 1 let fighterType be calculated based on the fighter's dendroidBool. (Or refactor by completely removing fighterType from the codebase to avoid this logical redundancy.)

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L530-L532

Vulnerability details

Impact

Points can be gained even when nothing is staked, with no risk of loss. These points earn the user claimable NRN tokens.

Proof of Concept

If nothing is staked and the fighter wins then the number of points credited to it and its owner is points = stakingFactor[tokenId] * eloFactor, where eloFactor > 0 is provided by the game server and stakingFactor[tokenId] = _getStakingFactor(tokenId, stakeAtRisk). _getStakingFactor() returns at least 1. Thus points >= 1 always, and even an unstaked win will gain points.

If the unstaked fighter loses then either points are lost as normal, or

bool success = _neuronInstance.transfer(_stakeAtRiskAddress, curStakeAtRisk);
if (success) {
    _stakeAtRiskInstance.updateAtRiskRecords(curStakeAtRisk, tokenId, fighterOwner);
    amountStaked[tokenId] -= curStakeAtRisk;
}

is executed, but curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4 is 0 since nothing has been staked so nothing is transferred and nothing is lost.

These points can then be converted to NRN through claimNRN().

This is thus a foolproof way to earn arbitrary amounts of NRN. Note also that since nothing is staked the fighter can be transferred freely and when it is the new owner has a fresh voltage and battles can therefore be initiated without limit.

Recommended Mitigation Steps

Let the range of _getStakingFactor() extend down to 0.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

When generation is incremented all fighters will be created with element = 0, except those minted from the merging pool, giving those an unfair advantage.

Proof of Concept

In FighterFarm.sol numElements is a mapping of the number elements by generation. numElements[0] = 3 by default in the constructor. It's only use is in _createFighterBase() to set element from dna: uint256 element = dna % numElements[generation[fighterType]];. generation is initialized as [0,0], but can be incremented by the owner in incrementGeneration(). It cannot be decremented. If it is incremented then numElements[generation[fighterType]] will return 0 and element will therefore invariably be set to 0.

element is set in this way whenever a fighter is created via claimFighters() or redeemMintPass(), or when changed via reRoll(). Only mintFromMergingPool() allows setting a custom element. This gives an unfair advantage to fighters minted from the merging pool, allowing them to have the element which has the relative advantage over element 0.

Recommended Mitigation Steps

Implement functionality which sets numElements, independently or when generation is incremented.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

Only a tiny fraction, $0.0002427 \%$, of all rarity rank combinations are possible. The probability distribution of the possible combinations (assuming the DNA is uniformly random) is not uniform: $24 \%$ of combinations are twice as likely as the rest.

Proof of Concept

AiArenaHelper.createPhysicalAttributes() may set six finalAttributeProbabilityIndexes using the following calculation.

uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
uint256 attributeIndex = dnaToIndex(generation, rarityRank, attributes[i]);
finalAttributeProbabilityIndexes[i] = attributeIndex;

attributeToDnaDivisor is by default [2, 3, 5, 7, 11, 13]. Each rarityRank by itself is in the range 0..99, and ostensibly the total number of combinations of the six rarityRanks should then be $100^6$. However, only $2,427,000$ different combinations are possible, which is only $0.0002427 \%$ of all combinations that should be possible.

As a function of dna the vector of the six rarityRanks repeats every $3,003,000$ integers ($=2 \cdot 3 \cdot 5 \cdot 7 \cdot 11 \cdot 13 \cdot 100$). But it turns out that $576,000$ of these appear twice. For example, a dna of 0 or 1 yield the same combination of rarityRanks ($[0,0,0,0,0,0]$), as do 16 and 17 ($[8,5,3,2,1,1]$), and 22 and 23 ($[11,7,4,3,2,1]$), etc. That is, about $24 \%$ of the combinations are twice as likely as the rest.

The rarityRanks are input to dnaToIndex() which then will also be biased and restricted in output (depending on the attribute probabilities).

Recommended Mitigation Steps

Extract multiple small random numbers by repeatedly taking the modulo and dividing. I.e.

- uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
+ uint256 rarityRank = dna % 100;
+ dna /= 100;

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

Claiming rewards in the merging pool, i.e. minting fighters from the merging pool, fails to perform checks that the custom attributes are valid numbers.

Proof of Concept

A fighter's element and weight are numbers, each representing one of a small set of features. These are set by FighterFarm._createNewFighter() using the customAttributes parameter. In most cases they are there calculated by FighterFarm._createFighterBase() (by passing customAttributes[0] = 100) which returns them in the ranges 0..2 (by default at least) and 65..95 respectively. However, when creating a fighter via mintFromMergingPool() (by the user's calling MergingPool.claimRewards()) the user can set his own customAttributes and no check is performed on these that they are within the valid ranges (provided customAttributes[0] != 100).

Recommended Mitigation Steps

Perform the requisite checks in _createNewFighter() or mintFromMergingPool().

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The user can transfer his token to an address generated by brute force to enable him to reRoll() a fighter with optimal DNA and thus optimal attributes. Or similarly possibly get a signed message for this address to claimFighters().

Proof of Concept

In FighterFarm.sol msg.sender is used to determine the DNA of the created fighter in claimFighters() and reRoll(). The DNA is used as the seed to a pseudo-random selection of the fighter's traits and attributes.

An address can be generated by brute force which would give a DNA which generates the desired and most advantageous attributes. The user can then simply get a signed message for this address or transfer his token to this address to be able to create a fighter with these attributes.

Whether this exploit is possible in the case of claimFighters() depends on the conditions on which the delegated signer signs the message, but no such obstacle applies to the case of reRoll().

Recommended Mitigation Steps

Use a seed which is not as easily influenceable by the user, such as Chainlink VRF, by block.prevrandao, or set by the delegated signer.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

The best out of the entire sequence of reroll results can be selected, instead of having to take a chance at each reroll attempt.

Proof of Concept

FighterFarm.reRoll() allows the holder of a fighter to pseudo-randomly set new traits for his fighter, for a cost, a limited number of times.

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);
    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] = "";
    }
}

The randomness is created in _createFighterBase() which is seeded by uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId]))). msg.sender, tokenId and numRerolls[tokenId] are knowable in advance for all rerolls attempted by the user. This means that he can precalculate the results of all of his allowed rerolls and select the best one, which gives him an advantage over the regular user who would, as is intended, have to submit to the mercy of each random roll.

Recommended Mitigation Steps

Set the DNA in some other unpredictable manner, ideally using Chainlink VRF, or by block.prevrandao.

Assessed type

Other