AI Arena - Tychai0s's results

Lines of code

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

Vulnerability details

Impact

This issue allows users to bypass re-roll limits for fighters by exploiting the absence of checks for fighterType in the re-roll function. This could lead to unfair advantages, imbalance in gameplay, and potential exploitation where certain fighters could be re-rolled more times than intended.

Proof of Concept

In FighterFarm.sol:370, the reRoll function lacks verification for fighterType, meaning it does not properly enforce re-roll limitations across different types of fighters. As numRerolls[tokenId] is checked against maxRerollsAllowed[fighterType], without validating if the fighterType corresponds to the tokenId's original type, users can potentially re-roll a fighter more times than allowed by simply specifying a different fighterType with a higher re-roll limit. This oversight can lead to gameplay imbalance and exploitation.

function reRoll(uint8 tokenId, uint8 fighterType) public {
    // Potential to bypass re-roll limits due to lack of fighterType checks
    require(msg.sender == ownerOf(tokenId));
    require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]);
    ...
}

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a validation mechanism within the reRoll function to ensure that the fighterType specified matches the fighterType of the tokenId being re-rolled. This could involve mapping each tokenId to its fighterType and checking this mapping within the function. Such a measure would prevent users from bypassing re-roll limits and maintain fair gameplay and balance.

Issue Type

Invalid Validation

Assessed type

Invalid Validation

Judge has assessed an item in Issue #1214 as 3 risk. The relevant finding follows:

Reroll Function Vulnerability in FighterFarm Contract Due to Missing FighterType Check

Lines of code

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

Vulnerability details

Impact

This vulnerability allows users to stake NRN tokens in a manner that fulfills the requirements for earning points without actually risking any stake. Users can exploit this by manipulating the stake calculation to always round down to zero, thus never transferring any stake to the StakeAtRisk contract, while still accruing points to increase their chances of earning new fighters.

Proof of Concept

In RankedBattle.sol:439, the method _addResultPoints can be exploited due to the way curStakeAtRisk is calculated. By staking an amount that makes curStakeAtRisk round down to 0 due to integer division, a user can avoid having any stake moved to the StakeAtRisk contract. This exploit is facilitated by the formula curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10 ** 4, where precise staking amounts can ensure curStakeAtRisk results in 0 after division. Consequently, the user can allocate all points to the mergingPool, increasing their chances of earning new fighters without any associated risk.

// RankedBattle.t.sol
function testUserCanGetStakingBenefitsByStakingDust() public {
        address staker = vm.addr(3);
        uint256 bpsPerLoss = _rankedBattleContract.bpsLostPerLoss();
        uint256 neuronToStake = 10 ** 4 / bpsPerLoss - 1;

        _mintFromMergingPool(staker);
        _fundUserWithNeuronByTreasury(staker, neuronToStake);

        vm.prank(staker);
        _rankedBattleContract.stakeNRN(neuronToStake, 0);

        vm.startPrank(_GAME_SERVER_ADDRESS);
        _rankedBattleContract.updateBattleRecord(0, 0, 2, 1500, false); // staker lost his match, but StakeAtRisk took 0 Neuron
        uint256 initialStakeAtRisk = _stakeAtRiskContract.getStakeAtRisk(0);
        assertEq(initialStakeAtRisk, 0);

        // staker enjoys a nice streak of wins, setting all of his points toward the merginPool (100)
        _rankedBattleContract.updateBattleRecord(0, 100, 0, 1500, false);
        _rankedBattleContract.updateBattleRecord(0, 100, 0, 1500, false);

        uint256 pointsAfterWins = _mergingPoolContract.fighterPoints(0);
        assertGt(pointsAfterWins, 0);

// Even if the user just keeps losing, no points nor stake will be at risk of being lost
        _rankedBattleContract.updateBattleRecord(0, 0, 2, 1500, false);
        _rankedBattleContract.updateBattleRecord(0, 0, 2, 1500, false);
        _rankedBattleContract.updateBattleRecord(0, 0, 2, 1500, false);
        _rankedBattleContract.updateBattleRecord(0, 0, 2, 1500, false);
        _rankedBattleContract.updateBattleRecord(0, 0, 2, 1500, false);

        uint256 updatedStakeAtRisk = _stakeAtRiskContract.getStakeAtRisk(0);
        assertEq(updatedStakeAtRisk, 0);

        uint256 pointsAfterLoses = _mergingPoolContract.fighterPoints(0);
        assertGt(pointsAfterLoses, 0);

        _rankedBattleContract.updateBattleRecord(0, 100, 0, 1500, false); // Win again
        assertGt(_mergingPoolContract.fighterPoints(0), pointsAfterWins);

        vm.stopPrank();
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a minimum stake requirement that ensures curStakeAtRisk cannot be rounded down to 0, potentially by setting a minimum threshold for bpsLostPerLoss. Consider using a more precise arithmetic library or techniques to prevent rounding errors that could be exploited.

Issue Type

Math

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

This design flaw restricts the re-rolling feature to only the first 256 fighters due to the tokenId parameter being defined as a uint8 instead of a uint256. This severely limits the game's scalability and the players' ability to engage with the re-roll mechanic for fighters beyond the initial 256.

Proof of Concept

In FighterFarm.sol:370, the reRoll function is implemented with a tokenId parameter of type uint8. This choice of data type inadvertently restricts the function to operate only on the first 256 tokens because uint8 can only represent values from 0 to 255. As a result, any attempt to re-roll fighters with tokenId values beyond 255 will fail due to this type limitation.

function reRoll(uint8 tokenId, uint8 fighterType) public {
    // Only the first 256 fighters can be re-rolled due to uint8 limitation
    require(msg.sender == ownerOf(tokenId));
    ...
}

Tools Used

Manual Review

Recommended Mitigation Steps

To rectify this limitation, it is recommended to change the data type of the tokenId parameter in the reRoll function from uint8 to uint256. This change will align the function with the ERC-721 standard's expectation for token identifiers and ensure that all tokens, regardless of their number, can be re-rolled as intended.

Issue Type

Invalid Validation

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

This issue leads to a panic revert whenever a fighter is rerolled or created with customAttributes[0] flag set to 100 while using a generation other than the first one, effectively halting the creation process due to a division by zero error. This can restrict the game's expansion to new generations of fighters, directly impacting the game's scalability and user experience.

Proof of Concept

In FighterFarm.sol:416, the _createFighterBase function calculates a fighter's element using the modulo operator with an uninitialized value in the numElements mapping for generations beyond the first. Since numElements for new generations is never set beyond the initial generation (index 0), any attempt to create a fighter or reroll an existing fighter with a subsequent generation will result in a division by zero error, causing the function to revert and fail to create a new fighter. This issue is critical as it prevents the successful creation of fighters for any generation beyond the first, thereby limiting the functionality and future expansion of the platform.

uint256 element = dna % numElements[generation[fighterType]]; // Division by zero error for generations > 0

// FighterFarm.t.sol
function testMintRevertsOnNewGenerations() public {
        _mintFromMergingPool(_ownerAddress);
        // get 4k neuron from treasury
        _fundUserWithNeuronByTreasury(_ownerAddress, 10_000 * 10 ** 18);

        _fighterFarmContract.incrementGeneration(0); // Increase generation count
        vm.prank(address(_mergingPoolContract));
        _fighterFarmContract.mintFromMergingPool(
            _ownerAddress, "_neuralNetHash", "original", [ /*customAttributes flag set*/ uint256(100), uint256(80)]
        ); // panic: division or modulo by 0

        /* Notice this will also happen when trying to reroll fighters
        if (_fighterFarmContract.ownerOf(0) == _ownerAddress) {
            _neuronContract.addSpender(address(_fighterFarmContract));
            _fighterFarmContract.reRoll(0, 0); // panic: division or modulo by 0
        }
        **/
    }

Tools Used

Manual Review

Recommended Mitigation Steps

To resolve this issue, implement a mechanism to initialize numElements for each new generation within the contract. This can be achieved by either providing a function that allows the contract owner or another authorized entity to set numElements for new generations or by implementing a default value for all generations. Ensuring numElements is properly set before any fighters of a new generation are created will prevent the division by zero error and allow the game to scale to new generations seamlessly.

Issue Type

Invalid Validation

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The absence of a mechanism to revoke the staker role in FighterFarm.sol could lead to permanent authorization for addresses once they are granted the ability to stake fighters. This flaw limits the control over who can participate in staking, potentially leading to security risks or abuse of the system if malicious actors or compromised accounts cannot be removed from the staking pool.

Proof of Concept

In FighterFarm.sol:82, the contract maintains a mapping (hasStakerRole) to track addresses authorized to stake fighters. However, it lacks a function or mechanism to revoke this staking ability once granted. This oversight means that once an address is authorized, it remains permanently able to stake fighters, with no way to remove this privilege in response to changes in game dynamics, security concerns, or the address's behavior.

mapping(address => bool) public hasStakerRole; // No mechanism to revoke this role once granted

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a function that allows contract administrators or designated roles to revoke the staker role from addresses. This function should securely modify the hasStakerRole mapping to reflect the revocation of staking privileges.

Assessed type

Access Control