AI Arena - SpicyMeatball's results

In AI Arena you train an AI character to battle in a platform fighting game. Imagine a cross between Pokรฉmon and Super Smash Bros, but the characters are AIs, and you can train them to learn almost any skill in preparation for battle.

General Information

Platform: Code4rena

Start Date: 09/02/2024

Pot Size: $60,500 USDC

Total HM: 17

Participants: 283

Period: 12 days

Judge:

Id: 328

League: ETH

AI Arena

Findings Distribution

Researcher Performance

Rank: 104/283

Findings: 11

Award: $59.67

๐ŸŒŸ Selected for report: 0

๐Ÿš€ Solo Findings: 0

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L291 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol#L120

Vulnerability details

Impact

Users can transfer non-transferable game items with safeBatchTransfer

Proof of Concept

Creator of the game item token can set transferable option to false and bind token to one account restricting any transfers.

    function createGameItem(
        string memory name_,
        string memory tokenURI,
        bool finiteSupply,
        bool transferable,
        uint256 itemsRemaining,
        uint256 itemPrice,
        uint16 dailyAllowance
    ) 
        public 
    {
        require(isAdmin[msg.sender]);
        allGameItemAttributes.push(
            GameItemAttributes(
                name_,
                finiteSupply,
>>              transferable,
                itemsRemaining,
                itemPrice,
                dailyAllowance
            )
        );

This restriction can be bypassed since only safeTransferFrom has a transferability check while safeBatchTransferFrom doesn't. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L301

Coded POC for GameItems.t.sol

    function testTransferSoulbound() public {
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _gameItemsContract.createGameItem("Soulbound", "https://ipfs.io/ipfs/", true, false, 10_000, 1 * 10 ** 18, 10);
        _gameItemsContract.mint(1, 1);

        vm.expectRevert();
        _gameItemsContract.safeTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, 1, 1, "");

        uint256[] memory ids = new uint256[](1);
        uint256[] memory values = new uint256[](1);
        ids[0] = 1;
        values[0] = 1;
        _gameItemsContract.safeBatchTransferFrom(_ownerAddress, _DELEGATED_ADDRESS, ids, values, "");
    }

Tools Used

Foundry

Restrict safeBatchTransferFrom as well

Assessed type

Token-Transfer

#0 - c4-pre-sort

2024-02-22T03:35:23Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-22T03:35:29Z

raymondfam marked the issue as duplicate of #18

#2 - c4-pre-sort

2024-02-26T00:27:25Z

raymondfam marked the issue as duplicate of #575

#3 - c4-judge

2024-03-05T04:47:39Z

HickupHH3 changed the severity to 3 (High Risk)

#4 - c4-judge

2024-03-05T04:51:21Z

HickupHH3 marked the issue as satisfactory

Awards

1.2667 USDC - $1.27

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
upgraded by judge
edited-by-warden
:robot:_86_group
duplicate-366

External Links

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L235-L236

Vulnerability details

Impact

User can mint rare type of fighters like dendroids and icons, which shouldn't be mintable.

Proof of Concept

According to docs https://docs.aiarena.io/gaming-competition/nft-makeup

๐Ÿ“บ Dendroids - Dendroids are a more exclusive class of NFTs. They have the ability to incorporate other metaverse assets (e.g. NFTs from other projects) into the Arena! These are very rare and more difficult to obtain. Youโ€™ll have to follow the story to find out how to get access to one of these!

Icons are special cosmetic items https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L83

    function createPhysicalAttributes(
        uint256 dna, 
        uint8 generation, 
        uint8 iconsType, 
        bool dendroidBool
    ) 
        external 
        view 
        returns (FighterOps.FighterPhysicalAttributes memory) 
    {
>>      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;

Both icons and dendroids are free to mint with redeemMintPass since attributes fighterTypes and iconTypes are not validated

    function redeemMintPass(
        uint256[] calldata mintpassIdsToBurn,
>>      uint8[] calldata fighterTypes,
>>      uint8[] calldata iconsTypes,
        string[] calldata mintPassDnas,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        require(
            mintpassIdsToBurn.length == mintPassDnas.length && 
            mintPassDnas.length == fighterTypes.length && 
            fighterTypes.length == modelHashes.length &&
            modelHashes.length == modelTypes.length
        );
        for (uint16 i = 0; i < mintpassIdsToBurn.length; i++) {
            require(msg.sender == _mintpassInstance.ownerOf(mintpassIdsToBurn[i]));
            _mintpassInstance.burn(mintpassIdsToBurn[i]);
            _createNewFighter(
                msg.sender, 
                uint256(keccak256(abi.encode(mintPassDnas[i]))), 
                modelHashes[i], 
                modelTypes[i],
>>              fighterTypes[i],
>>              iconsTypes[i],
                [uint256(100), uint256(100)]
            );
        }
    }

Coded POC for FighterFarm.t.sol

function testMintDendroid() public { uint8[2] memory numToMint = [1, 0]; bytes memory signature = abi.encodePacked( hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c" ); string[] memory _tokenURIs = new string[](1); _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m"; // first i have to mint an nft from the mintpass contract _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs); // once owning one i can then redeem it for a fighter uint256[] memory _mintpassIdsToBurn = new uint256[](1); string[] memory _mintPassDNAs = new string[](1); uint8[] memory _fighterTypes = new uint8[](1); uint8[] memory _iconsTypes = new uint8[](1); string[] memory _neuralNetHashes = new string[](1); string[] memory _modelTypes = new string[](1); _mintpassIdsToBurn[0] = 1; _mintPassDNAs[0] = "dna"; _neuralNetHashes[0] = "neuralnethash"; _modelTypes[0] = "original"; _iconsTypes[0] = 1; // Dendroid is fighterType = 1 _fighterTypes[0] = 1; // approve the fighterfarm contract to burn the mintpass _mintPassContract.approve(address(_fighterFarmContract), 1); _fighterFarmContract.redeemMintPass( _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes ); (, uint256[6] memory attr, , , , ,) = _fighterFarmContract.getAllFighterInfo(0); // Dendroids have all their attributes set to 99 assertEq(attr[0], 99); }

Tools Used

Foundry

If we look in the AAMintPass.sol we can see that users can mint passes for dendroids

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AAMintPass.sol#L109

    /// @notice This allows you to claim a mintpass which you have qualified for
    /// @dev Users must provide the number of mintpasses they want to claim, along with the 
    /// tokenURIs and a signature from our delegated server address. We then verify that the 
    /// server did indeed sign a message approving them to claim the amount of mint passes.
    /// We use passesClaimed as a part of the message and increment it to ensure they cannot use
    /// the same signature multiple times.
    /// @param numToMint The number of mintpasses to claim. The first element in the array is the
>>  /// number of AI Champion mintpasses and the second element is the number of Dendroid 
    /// mintpasses.
    /// @param signature The signature from the delegated server address
    /// @param _tokenURIs Token URIs for each of the mintpasses a user claims 
    function claimMintPass(
        uint8[2] calldata numToMint,
        bytes calldata signature,
        string[] calldata _tokenURIs
    ) 
        external 
    {

But there is no way to know which mint pass is the dendroid one, because of that we should consider removing ability to mint dendroids for now.

    function redeemMintPass(
        uint256[] calldata mintpassIdsToBurn,
-       uint8[] calldata fighterTypes,
-       uint8[] calldata iconsTypes,
        string[] calldata mintPassDnas,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        require(
            mintpassIdsToBurn.length == mintPassDnas.length && 
-           mintPassDnas.length == fighterTypes.length && 
            fighterTypes.length == modelHashes.length &&
            modelHashes.length == modelTypes.length
        );
        for (uint16 i = 0; i < mintpassIdsToBurn.length; i++) {
            require(msg.sender == _mintpassInstance.ownerOf(mintpassIdsToBurn[i]));
            _mintpassInstance.burn(mintpassIdsToBurn[i]);
            _createNewFighter(
                msg.sender, 
                uint256(keccak256(abi.encode(mintPassDnas[i]))), 
                modelHashes[i], 
                modelTypes[i],
+               0,
+               0,
                [uint256(100), uint256(100)]
            );
        }
    }

Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-02-22T07:13:35Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-22T07:13:45Z

raymondfam marked the issue as duplicate of #33

#2 - c4-pre-sort

2024-02-26T00:52:53Z

raymondfam marked the issue as duplicate of #1626

#3 - c4-judge

2024-03-05T10:53:04Z

HickupHH3 changed the severity to 3 (High Risk)

#4 - c4-judge

2024-03-05T10:53:08Z

HickupHH3 marked the issue as satisfactory

Lines of code

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

Vulnerability details

Impact

Lack of validation leads to multiple consequences

  • user can re-roll more times that is allowed for his actual fighter type
  • dendroids who re-roll as a common fighter will receive attributes that differ from their default [99, 99, 99, 99, 99, 99]
  • fighter can receive attributes based on probabilities from older generations

Proof of Concept

Every time function incrementGeneration is called maxRerollAllowed for a specified fighter type is increased by 1 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L129

    function incrementGeneration(uint8 fighterType) external returns (uint8) {
        require(msg.sender == _ownerAddress);
        generation[fighterType] += 1;
>>      maxRerollsAllowed[fighterType] += 1;
        return generation[fighterType];
    }

Because fighter type is not validated, user can re-roll more times that he is allowed, for example type 0 fighters current gen is 3 and type 1 gen is 0. Type 1 owner can cal reRoll with fighterType = 0 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L370

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

And re-roll two more times, the trade-off is that fighter will lose it's default dendroid appearance, but since it's just a cosmetic compared to the element and weight parameters, it's a pretty good deal.

Some tweaks are needed to run the test below

silence assertion in FighterFarm.t.sol https://github.com/code-423n4/2024-02-ai-arena/blob/main/test/FighterFarm.t.sol#L398

    /// @notice Helper function to fund an account with 4k $NRN tokens.
    function _fundUserWith4kNeuronByTreasury(address user) internal {
        vm.prank(_treasuryAddress);
        _neuronContract.transfer(user, 4_000 * 10 ** 18);
+       //assertEq(4_000 * 10 ** 18 == _neuronContract.balanceOf(user), true);
    }

silence the elements bug https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L462

    function _createFighterBase(
        uint256 dna, 
        uint8 fighterType
    ) 
        private 
        view 
        returns (uint256, uint256, uint256) 
    {
+       uint256 element = 1;//dna % numElements[generation[fighterType]];
        uint256 weight = dna % 31 + 65;
        uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);
        return (element, weight, newDna);
    }

Coded POC for FighterFarm.t.sol

    function testRerollExploit() public {
        // get 4k neuron from treasury
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _neuronContract.addSpender(address(_fighterFarmContract));

        uint8[2] memory numToMint = [1, 0];
        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        // first i have to mint an nft from the mintpass contract
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);

        // once owning one i can then redeem it for a fighter
        uint256[] memory _mintpassIdsToBurn = new uint256[](1);
        string[] memory _mintPassDNAs = new string[](1);
        uint8[] memory _fighterTypes = new uint8[](1);
        uint8[] memory _iconsTypes = new uint8[](1);
        string[] memory _neuralNetHashes = new string[](1);
        string[] memory _modelTypes = new string[](1);

        _mintpassIdsToBurn[0] = 1;
        _mintPassDNAs[0] = "dna";
        _neuralNetHashes[0] = "neuralnethash";
        _modelTypes[0] = "original";
        _iconsTypes[0] = 1;
  
        // Dendroid is fighterType = 1
        _fighterTypes[0] = 1;

        // approve the fighterfarm contract to burn the mintpass
        _mintPassContract.approve(address(_fighterFarmContract), 1);

        _fighterFarmContract.redeemMintPass(
            _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
        );
        
        // Increase generation for type 0
        _fighterFarmContract.incrementGeneration(0);
        _fighterFarmContract.incrementGeneration(0);
        // check the roll count
        assertEq(_fighterFarmContract.maxRerollsAllowed(0), 5);
        assertEq(_fighterFarmContract.maxRerollsAllowed(1), 3);

        // normal re-roll
        _fighterFarmContract.reRoll(0, 1);
        _fighterFarmContract.reRoll(0, 1);
        _fighterFarmContract.reRoll(0, 1);
        // revert max reroll allowance exceeded
        vm.expectRevert();
        _fighterFarmContract.reRoll(0, 1);
        // re-roll with other type as an argument
        _fighterFarmContract.reRoll(0, 0);
        _fighterFarmContract.reRoll(0, 0);
    }

Tools Used

Foundry

Assert that tokenId fighter type is the same as fighterType

    function reRoll(uint8 tokenId, uint8 fighterType) public {
+        require((fighterType == 1) == fighters[tokenId].dendroidBool, "Type mismatch");
        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] = "";
        }
    }   

Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-02-21T23:55:52Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-21T23:56:15Z

raymondfam marked the issue as duplicate of #17

#2 - c4-pre-sort

2024-02-22T00:36:34Z

raymondfam marked the issue as not a duplicate

#3 - c4-pre-sort

2024-02-22T00:36:39Z

raymondfam marked the issue as sufficient quality report

#4 - c4-pre-sort

2024-02-22T00:36:44Z

raymondfam marked the issue as primary issue

#5 - raymondfam

2024-02-22T00:40:33Z

This report covers three consequences from the same root cause of fighter type validation: 1. more re-rolls, 2. rarer attribute switch, 3. generation attribute switch.

#6 - c4-pre-sort

2024-02-22T00:58:13Z

raymondfam marked the issue as duplicate of #306

#7 - c4-judge

2024-03-05T04:26:44Z

HickupHH3 marked the issue as satisfactory

#8 - c4-judge

2024-03-19T09:05:08Z

HickupHH3 changed the severity to 3 (High Risk)

Lines of code

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

Vulnerability details

Impact

It will be impossible to create fighters if generation is incremented

Proof of Concept

When a user mints or re-rolls a fighter token _createFighterBase function is invoked https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L380 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L500

    function _createFighterBase(
        uint256 dna, 
        uint8 fighterType
    ) 
        private 
        view 
        returns (uint256, uint256, uint256) 
    {
>>      uint256 element = dna % numElements[generation[fighterType]];
        uint256 weight = dna % 31 + 65;
        uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);
        return (element, weight, newDna);
    }

We use the number of elements for a given generation ,that is stored in the elements mapping, to decide the fighter element. elements[0] is initialized in the constructor.

    constructor(address ownerAddress, address delegatedAddress, address treasuryAddress_)
        ERC721("AI Arena Fighter", "FTR")
    {
        _ownerAddress = ownerAddress;
        _delegatedAddress = delegatedAddress;
        treasuryAddress = treasuryAddress_;
>>      numElements[0] = 3;
    } 

Unfortunately later, when the owner calls incrementGeneration function to change generation of the specified fighter type

    function incrementGeneration(uint8 fighterType) external returns (uint8) {
        require(msg.sender == _ownerAddress);
>>      generation[fighterType] += 1;
        maxRerollsAllowed[fighterType] += 1;
        return generation[fighterType];
    }

elements value is not updated for the next generation and will hold a zero, as a result all attempts to create a fighter of the new generation will revert.

Here is the coded POC for FighterFarm.t.sol

    function testElementsNotUpdated() public {
        assertEq(_fighterFarmContract.numElements(0), 3);
        // increment generation for fighter type 0
        _fighterFarmContract.incrementGeneration(0);
        // generation is set to 1
        assertEq(_fighterFarmContract.generation(0), 1);
        // 0 elements 
        assertEq(_fighterFarmContract.numElements(1), 0);
        
        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";

        // will revert with "Division or modulo by 0"
        _fighterFarmContract.claimFighters(numToMint, claimSignature, claimModelHashes, claimModelTypes);
    }

Tools Used

Foundry

Update elements mapping in incrementGeneration

    function incrementGeneration(uint8 fighterType) external returns (uint8) {
        require(msg.sender == _ownerAddress);
+       uint8 currGeneration = generation[fighterType];
        generation[fighterType] += 1;
        maxRerollsAllowed[fighterType] += 1;
+       if(elements[currGeneration + 1] == 0)
+           elements[currGeneration] = 3;
        return generation[fighterType];
    }

Assessed type

DoS

#0 - c4-pre-sort

2024-02-22T18:19:08Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-22T18:19:23Z

raymondfam marked the issue as duplicate of #45

#2 - c4-judge

2024-03-07T06:53:22Z

HickupHH3 marked the issue as satisfactory

Awards

14.5737 USDC - $14.57

Labels

2 (Med Risk)
partial-50
duplicate-1507

External Links

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

Default admin is not initialized

#0 - c4-judge

2024-03-20T02:41:12Z

HickupHH3 marked the issue as duplicate of #1507

#1 - c4-judge

2024-03-20T02:41:17Z

HickupHH3 marked the issue as partial-50

Awards

1.876 USDC - $1.88

Labels

bug
2 (Med Risk)
insufficient quality report
satisfactory
:robot:_30_group
duplicate-932

External Links

Lines of code

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

Vulnerability details

Impact

The user can specify an element and weight that is far beyond the allowed boundaries.

Proof of Concept

The round winner can mint a fighter token as a reward from the MergingPool.sol

    function claimRewards(
        string[] calldata modelURIs, 
        string[] calldata modelTypes,
>>      uint256[2][] calldata customAttributes
    ) 
        external 
    {
        uint256 winnersLength;
        uint32 claimIndex = 0;
        uint32 lowerBound = numRoundsClaimed[msg.sender];
        for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
            numRoundsClaimed[msg.sender] += 1;
            winnersLength = winnerAddresses[currentRound].length;
            for (uint32 j = 0; j < winnersLength; j++) {
                if (msg.sender == winnerAddresses[currentRound][j]) {
                    _fighterFarmInstance.mintFromMergingPool(
                        msg.sender,
                        modelURIs[claimIndex],
                        modelTypes[claimIndex],
>>                      customAttributes[claimIndex]
                    );
                    claimIndex += 1;
                }
            }
        }
        if (claimIndex > 0) {
            emit Claimed(msg.sender, claimIndex);
        }
    }

Function claimRewards accept any values for customAttributes, later these attributes are used to initialize the element and weight of the fighter https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L329

    function _createNewFighter(
        address to, 
        uint256 dna, 
        string memory modelHash,
        string memory modelType, 
        uint8 fighterType,
        uint8 iconsType,
        uint256[2] memory customAttributes
    ) 
        private 
    {  
        require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);
        uint256 element; 
        uint256 weight;
        uint256 newDna;
        if (customAttributes[0] == 100) {
            (element, weight, newDna) = _createFighterBase(dna, fighterType);
        }
        else {
>>          element = customAttributes[0];
>>          weight = customAttributes[1];
            newDna = dna;
        }

Although there are certain boundaries that are set for the element and weight values, https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L470-L471

        uint256 element = dna % numElements[generation[fighterType]];
        uint256 weight = dna % 31 + 65;

they are bypassed if we use merging pool to mint a fighter. Since it is unknown how backend will process values that are outside the limits, I rate the severity of this bug as medium.

Coded POC for FighterFarm.t.sol

    function testCustomAttributes() public {
        vm.startPrank(address(_mergingPoolContract));
        uint256 MAX = type(uint256).max;
        _fighterFarmContract.mintFromMergingPool(_ownerAddress, "_neuralNetHash", "original", [MAX, MAX]);
        // let's see if we got the hat
        (, , uint256 weight, uint256 element, , ,) = _fighterFarmContract.getAllFighterInfo(0);
        assertEq(weight, MAX);
        assertEq(element, MAX);
    }

Tools Used

Foundry

Validate customAttributes

Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-02-24T08:51:36Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-24T08:51:48Z

raymondfam marked the issue as duplicate of #226

#2 - c4-judge

2024-03-11T10:19:04Z

HickupHH3 marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/MergingPool.sol#L148-L163 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L495

Vulnerability details

Impact

Winner can't claim his reward from the merging pool if the reward exceeds MAX_FIGHTERS_ALLOWED = 10.

Proof of Concept

Every round the merging pool admin decides a certain amount of winners, these addresses can mint a fighter token as a reward

    function claimRewards(
        string[] calldata modelURIs, 
        string[] calldata modelTypes,
        uint256[2][] calldata customAttributes
    ) 
        external 
    {
        uint256 winnersLength;
        uint32 claimIndex = 0;
        uint32 lowerBound = numRoundsClaimed[msg.sender];
        for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
            numRoundsClaimed[msg.sender] += 1;
            winnersLength = winnerAddresses[currentRound].length;
            for (uint32 j = 0; j < winnersLength; j++) {
                if (msg.sender == winnerAddresses[currentRound][j]) {
>>                  _fighterFarmInstance.mintFromMergingPool(
                        msg.sender,
                        modelURIs[claimIndex],
                        modelTypes[claimIndex],
                        customAttributes[claimIndex]
                    );
                    claimIndex += 1;
                }
            }
        }

A situation may arise when a user hasn't claimed his rewards for a long time, and as a result, the amount of claimable tokens has become greater than the maximum allowed fighters amount for one address.

Coded POC for MerginPool.t.sol

    function testClaimRewardsFail() public {
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_DELEGATED_ADDRESS);
        // 10 round winning streak just to prove the bug, in reality there will be other winners
        uint256[] memory _winners = new uint256[](2);
        _winners[0] = 0;
        _winners[1] = 1;
        string[] memory _modelURIs = new string[](10);
        string[] memory _modelTypes = new string[](10);
        uint256[2][] memory _customAttributes = new uint256[2][](10);
        for(uint256 i=0; i<10; i++) {
            _modelURIs[i] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
            _modelTypes[i] = "original";
            _customAttributes[i][0] = uint256(1);
            _customAttributes[i][1] = uint256(80);
            _mergingPoolContract.pickWinner(_winners);
        }
        // winner claims rewards for previous roundIds
        vm.expectRevert();
        _mergingPoolContract.claimRewards(_modelURIs, _modelTypes, _customAttributes);
    }

Tools Used

Foundry

Allow user to choose how many rewards he wants to claim in one transaction.

Assessed type

Token-Transfer

#0 - c4-pre-sort

2024-02-22T08:38:51Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-22T08:41:18Z

raymondfam marked the issue as sufficient quality report

#2 - c4-pre-sort

2024-02-22T08:41:31Z

raymondfam marked the issue as duplicate of #216

#3 - c4-judge

2024-03-11T12:48:32Z

HickupHH3 marked the issue as satisfactory

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

The user risks receiving an OOG error if he doesnโ€™t claim NRN rewards for too long.

#0 - c4-judge

2024-03-12T02:58:34Z

HickupHH3 marked the issue as duplicate of #216

#1 - c4-judge

2024-03-12T02:58:38Z

HickupHH3 marked the issue as partial-25

#2 - c4-judge

2024-03-21T03:03:25Z

HickupHH3 marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L107 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L214 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L254 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L324 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L379

Vulnerability details

Impact

The user can easily predict which dna value will give his fighter rare attributes and adjust the input accordingly. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L510

        FighterOps.FighterPhysicalAttributes memory attrs = _aiArenaHelperInstance.createPhysicalAttributes(
>>          newDna,
            generation[fighterType],
            iconsType,
            dendroidBool
        );

Proof of Concept

There are three ways to create a fighter in the FighterFarm contract:

  1. Mint with a signature from the backend
  2. Burn a mint pass
  3. Mint as a reward from the merging pool

Physical attributes of the newly created fighter depend on the dna value. First we calculate a rarity value from the dna that is a number from 0 to 100. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L107

    function createPhysicalAttributes(
        uint256 dna, 
        uint8 generation, 
        uint8 iconsType, 
        bool dendroidBool
    ) 
        external 
        view 
        returns (FighterOps.FighterPhysicalAttributes memory) 
    {
           ...
                } else {
>>                  uint256 rarityRank = (dna / attributeToDnaDivisor[attributes[i]]) % 100;
>>                  uint256 attributeIndex = dnaToIndex(generation, rarityRank, attributes[i]);
                    finalAttributeProbabilityIndexes[i] = attributeIndex;
                }
            }

Then we use rarity and probability array to decide the attribute

    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;
    }

For example, if we have probability array [60, 30, 10], to get third item that has probability 10%, we need to roll numbers 90-100 ( 90<= rarity <=100).

Now to our minter functions, let's start with the easiest one redeemMintPass https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L233-L262 User can directly input the dna value

    function redeemMintPass(
        uint256[] calldata mintpassIdsToBurn,
        uint8[] calldata fighterTypes,
        uint8[] calldata iconsTypes,
        string[] calldata mintPassDnas,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        ...
        for (uint16 i = 0; i < mintpassIdsToBurn.length; i++) {
            require(msg.sender == _mintpassInstance.ownerOf(mintpassIdsToBurn[i]));
            _mintpassInstance.burn(mintpassIdsToBurn[i]);
            _createNewFighter(
                msg.sender, 
>>              uint256(keccak256(abi.encode(mintPassDnas[i]))), 
                modelHashes[i], 
                modelTypes[i],
                fighterTypes[i],
                iconsTypes[i],
                [uint256(100), uint256(100)]
            );
        }
    }

Let's say I want that rare hat with 5% probability for my fighter, probability array is [5, 25, 25, 50], and divisor for the head attribute is 2. All I need is to find a string that will produce a hash (hash / 2) % 100 = 0..5.

Other methods are more tricky and use msg.sender and fighters array length to calculate the dna, but they still can be manipulated to some extent, which I will demonstrate in the coded POC. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L214 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L324 This one is a re-roll https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L379

Coded POC for FighterFarm.t.sol

redeemMintPass

    function testGetTheCoolHat() public {
        uint8[2] memory numToMint = [1, 0];
        bytes memory signature = abi.encodePacked(
            hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
        );
        string[] memory _tokenURIs = new string[](1);
        _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

        // first i have to mint an nft from the mintpass contract
        _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);

        // once owning one i can then redeem it for a fighter
        uint256[] memory _mintpassIdsToBurn = new uint256[](1);
        string[] memory _mintPassDNAs = new string[](1);
        uint8[] memory _fighterTypes = new uint8[](1);
        uint8[] memory _iconsTypes = new uint8[](1);
        string[] memory _neuralNetHashes = new string[](1);
        string[] memory _modelTypes = new string[](1);

        _mintpassIdsToBurn[0] = 1;
        _fighterTypes[0] = 0;
        _neuralNetHashes[0] = "neuralnethash";
        _modelTypes[0] = "original";
        _iconsTypes[0] = 1;
  
        // head probabilities array [25, 25, 13, 13, 9, 9]
        // let's get that 9% item, we need to roll 85-100
        uint256 coolHat = 6;
        // this will do
        _mintPassDNAs[0] = "blahblahblah";
        uint256 hash = uint256(keccak256(abi.encode(_mintPassDNAs[0])));
        console.log("DNA: ", hash);
        console.log("RAR: ", (hash / 2) % 100);        

        // approve the fighterfarm contract to burn the mintpass
        _mintPassContract.approve(address(_fighterFarmContract), 1);

        _fighterFarmContract.redeemMintPass(
            _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
        );
        // let's see if we got the hat
        (, uint256[6] memory attr, , , , ,) = _fighterFarmContract.getAllFighterInfo(0);
        assertEq(attr[0], coolHat);
    }

mintFromMergingPool

    function testGetTheCoolHatPartDeux() public {
        // head probabilities array [25, 25, 13, 13, 9, 9]
        // let's get that 9% item, we need to roll 85-100
        uint256 coolHat = 6;
        // this one is harder, msg.sender is always the same = merging pool, so we need to rely on figters array length
        // length of 2 is good!
        uint256 hash = uint256(keccak256(abi.encode(address(_mergingPoolContract), 2)));
        console.log("DNA: ", hash);
        console.log("RAR: ", (hash / 2) % 100);        

        vm.startPrank(address(_mergingPoolContract));
        // wait for other people to mint 2 tokens
        _fighterFarmContract.mintFromMergingPool(_ownerAddress, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
        _fighterFarmContract.mintFromMergingPool(_ownerAddress, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
        // now it's our turn
        _fighterFarmContract.mintFromMergingPool(_ownerAddress, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
        // let's see if we got the hat
        (, uint256[6] memory attr, , , , ,) = _fighterFarmContract.getAllFighterInfo(2);
        assertEq(attr[0], coolHat);
    }

Tools Used

Foundry

Make rarity value harder to predict, ideally chainlink VRF should be used, but if it's not possible we need to throw more parameters into the keccak256 algorithm.

For example

    function mintFromMergingPool(
        address to, 
        string calldata modelHash, 
        string calldata modelType, 
        uint256[2] calldata customAttributes
    ) 
        public 
    {
        require(msg.sender == _mergingPoolAddress);
        _createNewFighter(
            to, 
+           uint256(keccak256(abi.encode(msg.sender, fighters.length, to, modelHash, modelType, customAttributes))), 
            modelHash, 
            modelType,
            0,
            0,
            customAttributes
        );
    }

Assessed type

Other

#0 - c4-pre-sort

2024-02-24T01:28:29Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-24T01:28:54Z

raymondfam marked the issue as duplicate of #53

#2 - c4-judge

2024-03-06T03:47:46Z

HickupHH3 marked the issue as satisfactory

#3 - c4-judge

2024-03-06T03:49:26Z

HickupHH3 changed the severity to 3 (High Risk)

#4 - c4-judge

2024-03-20T01:04:09Z

HickupHH3 marked the issue as duplicate of #376

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/StakeAtRisk.sol#L102-L104 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L461-L462

Vulnerability details

Impact

If amountLost[user] is smaller than stakeAtRisk[roundId][fighterId], NRN reclaim is not possible, and as a result record update for the win condition will always revert.

Proof of Concept

Imagine a situation: Alice staked 1000 NRN to the fighter #1, after a sequence of defeats she ended up with 500 NRN at risk. Later, she decided to unstake what was left and sell her fighter. Bob buys it from her in hopes of receiving NRN that is currently at risk for himself.

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/StakeAtRisk.sol#L45-L49 The current state in round 0 is: amountLost[bob] = 0, stakeAtRisk[0][1] = 500.

Bob wins his first match with the fighter #1 and server calls the updateBattleRecord function https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L322

    function updateBattleRecord(
        uint256 tokenId, 
        uint256 mergingPortion,
        uint8 battleResult,
        uint256 eloFactor,
        bool initiatorBool
    ) 
        external 
    {   
        require(msg.sender == _gameServerAddress);
        require(mergingPortion <= 100);
        address fighterOwner = _fighterFarmInstance.ownerOf(tokenId);
        require(
            !initiatorBool ||
            _voltageManagerInstance.ownerVoltageReplenishTime(fighterOwner) <= block.timestamp || 
            _voltageManagerInstance.ownerVoltage(fighterOwner) >= VOLTAGE_COST
        );

        _updateRecord(tokenId, battleResult);
        uint256 stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);
>>      if (amountStaked[tokenId] + stakeAtRisk > 0) {
>>          _addResultPoints(battleResult, tokenId, eloFactor, mergingPortion, fighterOwner);
        }
        if (initiatorBool) {
            _voltageManagerInstance.spendVoltage(fighterOwner, VOLTAGE_COST);
        }
        totalBattles += 1;
    }

Stake at risk is > 0, contract will try to call _addResultPoints. Unfortunately this internal call will revert with underflow because risk values for token and owner are not the same https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L460-L461

    function reclaimNRN(uint256 nrnToReclaim, uint256 fighterId, address fighterOwner) external {
            ...
>>          stakeAtRisk[roundId][fighterId] -= nrnToReclaim;
            totalStakeAtRisk[roundId] -= nrnToReclaim;
>>          amountLost[fighterOwner] -= nrnToReclaim;
            emit ReclaimedStake(fighterId, nrnToReclaim);
        }
    }

Coded POC for RankedBattle.t.sol

    function testWinFail() public {
        address alice = vm.addr(3);
        address bob = vm.addr(4);
        _mintFromMergingPool(alice);
        uint8 tokenId = 0;
        _fundUserWith4kNeuronByTreasury(alice);
        vm.prank(alice);
        _rankedBattleContract.stakeNRN(1_000 * 10 ** 18, tokenId);
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(tokenId, 50, 2, 1500, true);
        uint256 atRisk = _stakeAtRiskContract.getStakeAtRisk(tokenId);
        assertEq(atRisk, 1e18);
        // alice unstakes and sells the token
        vm.startPrank(alice);
        _rankedBattleContract.unstakeNRN(999 * 10 ** 18, tokenId);
        _fighterFarmContract.transferFrom(alice, bob, tokenId);
        vm.stopPrank();
        // bob's win reverts with underflow
        vm.prank(address(_GAME_SERVER_ADDRESS));
        vm.expectRevert();
        _rankedBattleContract.updateBattleRecord(tokenId, 50, 0, 1500, true);
    }

Tools Used

Foundry

Perhaps we can modify reclaimNRN function

    function reclaimNRN(uint256 nrnToReclaim, uint256 fighterId, address fighterOwner) external {
        require(msg.sender == _rankedBattleAddress, "Call must be from RankedBattle contract");
        require(
            stakeAtRisk[roundId][fighterId] >= nrnToReclaim, 
            "Fighter does not have enough stake at risk"
        );

        bool success = _neuronInstance.transfer(_rankedBattleAddress, nrnToReclaim);
        if (success) {
            stakeAtRisk[roundId][fighterId] -= nrnToReclaim;
            totalStakeAtRisk[roundId] -= nrnToReclaim;
+           if(amountLost[fighterOwner] < nrnToReclaim) 
+               amountLost[fighterOwner] = 0;
            emit ReclaimedStake(fighterId, nrnToReclaim);
        }
    }

Assessed type

DoS

#0 - c4-pre-sort

2024-02-24T04:15:36Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-24T04:16:00Z

raymondfam marked the issue as duplicate of #1641

#2 - c4-judge

2024-03-12T04:01:25Z

HickupHH3 changed the severity to 3 (High Risk)

#3 - c4-judge

2024-03-12T04:03:31Z

HickupHH3 changed the severity to 2 (Med Risk)

#4 - c4-judge

2024-03-12T06:45:01Z

HickupHH3 marked the issue as satisfactory

Awards

29.6169 USDC - $29.62

Labels

bug
2 (Med Risk)
insufficient quality report
partial-50
:robot:_50_group
duplicate-43

External Links

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L312-L314

Vulnerability details

Impact

The user can change address and mint tokens beyond daily allowance that is set for one account.

Proof of Concept

Users are allowed to mint only certain amount of tokens each day, this allowance is replenished after 24 hours.

    function mint(uint256 tokenId, uint256 quantity) external {
        require(tokenId < _itemCount);
        uint256 price = allGameItemAttributes[tokenId].itemPrice * quantity;
        require(_neuronInstance.balanceOf(msg.sender) >= price, "Not enough NRN for purchase");
        require(
            allGameItemAttributes[tokenId].finiteSupply == false || 
            (
                allGameItemAttributes[tokenId].finiteSupply == true && 
                quantity <= allGameItemAttributes[tokenId].itemsRemaining
            )
        );
        require(
>>          dailyAllowanceReplenishTime[msg.sender][tokenId] <= block.timestamp || 
>>          quantity <= allowanceRemaining[msg.sender][tokenId]
        );

Unfortunately this restriction is pretty easy to bypass by calling the mint function from another address.

Coded POC for GameItems.t.sol

    function testBypassAllowance() public {
        address anotherAddress = address(0x2);
        assertEq(_gameItemsContract.getAllowanceRemaining(_ownerAddress, 0), 10);
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _fundUserWith4kNeuronByTreasury(anotherAddress);
        // mint from account 1
        _gameItemsContract.mint(0, 10);
        assertEq(_gameItemsContract.getAllowanceRemaining(_ownerAddress, 0), 0);
        // and again from another account, 20 tokens instead of 10
        vm.prank(anotherAddress);
        _gameItemsContract.mint(0, 10);
    }

Tools Used

Foundry

Use a global allowance counter instead of a personal one.

Assessed type

Token-Transfer

#0 - c4-pre-sort

2024-02-22T17:55:55Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-22T17:56:02Z

raymondfam marked the issue as duplicate of #43

#2 - c4-judge

2024-03-07T04:15:24Z

HickupHH3 marked the issue as partial-50

transferOwnership doesn't revoke admin rights from the old owner

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/VoltageManager.sol#L64 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/Neuron.sol#L85 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L108 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/MergingPool.sol#L89 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L167

The owner is granted admin rights when the contract is created

    constructor(address ownerAddress, address gameItemsContractAddress) {
        _ownerAddress = ownerAddress;
        _gameItemsContractInstance = GameItems(gameItemsContractAddress);
>>      isAdmin[_ownerAddress] = true;
    } 

When transferring ownership to another address, the old owner still retains that role. Consider setting isAdmin for the old owner to false.

User can bypass voltage limit by participating in battles from different addresses

Initiator of the ranked battle needs to spend 10 voltage units to participate https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L345-L347

    function spendVoltage(address spender, uint8 voltageSpent) public {
        require(spender == msg.sender || allowedVoltageSpenders[msg.sender]);
        if (ownerVoltageReplenishTime[spender] <= block.timestamp) {
            _replenishVoltage(spender);
        }
        ownerVoltage[spender] -= voltageSpent;
        emit VoltageRemaining(spender, ownerVoltage[spender]);
    }

The voltage is replenished after 24 hours or by sacrificing a special NFT https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/VoltageManager.sol#L117

    function _replenishVoltage(address owner) private {
>>      ownerVoltage[owner] = 100;
        ownerVoltageReplenishTime[owner] = uint32(block.timestamp + 1 days);
    }  

As we can see, the voltage is bound to the fighter owner's address, thus if owner has multiple fighters, he can split them between his wallets, increasing the number of matches he can participate in during the day. This includes staked and unstaked matches.

Signature can be replayed in different chains

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L191-L206 Users are allowed to mint a fighter token by providing a signature from an address owned by the protocol. If Ai-Arena will deploy in other networks in the future, this signature can be replayed.

FighterFarm.sol lacks setter for the _delegatedAddress

_delegatedAddress is used to sign fighter claim messages and setting token URIs, but unlike other instances such as NeuronToken or MergingPool, it cannot be changed.

The probability array is initialized twice

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

    constructor(uint8[][] memory probabilities) {
        _ownerAddress = msg.sender;

        // Initialize the probabilities for each attribute
>>      addAttributeProbabilities(0, probabilities);

        uint256 attributesLength = attributes.length;
>>      for (uint8 i = 0; i < attributesLength; i++) {
            attributeProbabilities[0][attributes[i]] = probabilities[i];
            attributeToDnaDivisor[attributes[i]] = defaultAttributeDivisor[i];
        }
    } 

addAttributeProbabilities(0, probabilities) performs the same function as the for loop below.

No 0 divisor check in addAttributeDivisor function

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L74 Since we are using elements attributeToDnaDivisor elements as divisors, additional check is needed to validate that these elements !=0

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

addAttributeProbabilities can use more validation

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/AiArenaHelper.sol#L131 attributeProbabilities elements correspond to the likelihood of acquiring a particular physical attribute. If the sum of the elements exceeds 100, some attributes will not be available.

        for (uint8 i = 0; i < attrProbabilitiesLength; i++) {
            cumProb += attrProbabilities[i];
            if (cumProb >= rarityRank) { // rarityRank is a number 0-100
                attributeProbabilityIndex = i + 1;
                break;
            }
        }

viewFighterInfo can be more verbose

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterOps.sol#L79 viewFighterInfo function doesn't return the fighter type or icon type.

Default admin is not initialized

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/Neuron.sol#L68-L76 Neuron token inherits from the OZ AccessControl contract, instead of using DEFAULT_ADMIN_ROLE it relies on custom functions like the one below

    function addMinter(address newMinterAddress) external {
        require(msg.sender == _ownerAddress);
>>      _setupRole(MINTER_ROLE, newMinterAddress);
    }

This makes role management less flexible because there is no way to revoke roles if needed. Consider granting DEFAULT_ADMIN_ROLE to the owner in the constructor.

setupAirdrop can overwrite unclaimed approvals

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/Neuron.sol#L127-L134

    function setupAirdrop(address[] calldata recipients, uint256[] calldata amounts) external {
        require(isAdmin[msg.sender]);
        require(recipients.length == amounts.length);
        uint256 recipientsLength = recipients.length;
        for (uint32 i = 0; i < recipientsLength; i++) {
>>          _approve(treasuryAddress, recipients[i], amounts[i]);
        }
    }

If the airdrop consists of multiple stages, users who do not claim their tokens using claim function risk having their approval overwritten in the next stage.

    // OZ ERC20 contract
    function _approve(
        address owner,
        address spender,
        uint256 amount
    ) internal virtual {
        require(owner != address(0), "ERC20: approve from the zero address");
        require(spender != address(0), "ERC20: approve to the zero address");

>>      _allowances[owner][spender] = amount;
        emit Approval(owner, spender, amount);
    }

Consider using increaseAllowance instead.

burnFrom shouldn't decrease allowance in case it's type(uint256).max

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/Neuron.sol#L201 If spender has an infinite allowance we can burn tokens without changing _allowances value.

Unusual setter for NRN distribution

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L220 setRankedNrnDistribution takes the NRN amount without the 10 ** 18 multiplier, most Dapps use X * 10 ** (decimal) amounts as arguments and it's pretty easy to make a mistake and enter such a value into setRankedNrnDistribution too.

globalStake value is not updated if staker loses his stake in battle

globalStake variable is used to track total amount of NRN tokens that was staked in the RankedBattle.sol contract. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L257 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L276 Unfortunately it's is not updated when part of the stake that was at risk during a round is transferred to the treasury at the end of the round. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L491-L498

            } else {
                /// If the fighter does not have any points for this round, NRNs become at risk of being lost
                bool success = _neuronInstance.transfer(_stakeAtRiskAddress, curStakeAtRisk);
                if (success) {
                    _stakeAtRiskInstance.updateAtRiskRecords(curStakeAtRisk, tokenId, fighterOwner);
                    amountStaked[tokenId] -= curStakeAtRisk;
                }
            }
        }

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/StakeAtRisk.sol#L142

    /// @notice Sweeps the lost stake to the treasury contract.
    /// @dev This function is called internally to transfer the lost stake to the treasury contract.
    function _sweepLostStake() private returns(bool) {
        return _neuronInstance.transfer(treasuryAddress, totalStakeAtRisk[roundId]);
    }

The user risks receiving an OOG error if he doesn't claim NRN rewards for too long.

NOT MENTIONED IN https://github.com/code-423n4/2024-02-ai-arena/blob/main/bot-report.md#l-05-external-calls-in-an-un-bounded-for-loop-may-result-in-a-dos

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L299-L305 claimNRN function iterates through all rounds in which user didn't claim to calculate the final reward, if a substantial number of rounds has passed since the last claim, the function can revert with an out of gas error.

        uint32 lowerBound = numRoundsClaimed[msg.sender];
        for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
            nrnDistribution = getNrnDistribution(currentRound);
            claimableNRN += (
                accumulatedPointsPerAddress[msg.sender][currentRound] * nrnDistribution   
            ) / totalAccumulatedPoints[currentRound];
            numRoundsClaimed[msg.sender] += 1;
        }

Consider allowing the user to specify the number of rounds for which he would like to be rewarded.

#0 - raymondfam

2024-02-26T07:29:35Z

The user risks receiving an OOG error if he doesnโ€™t claim NRN rewards for too long.: to #1541

#1 - c4-pre-sort

2024-02-26T07:29:40Z

raymondfam marked the issue as sufficient quality report

#2 - HickupHH3

2024-03-08T03:54:47Z

#267: L

#3 - c4-judge

2024-03-14T07:24:34Z

HickupHH3 marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax ยฉ 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter