AI Arena - VAD37'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: 86/283

Findings: 8

Award: $65.83

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Awards

1.2667 USDC - $1.27

Labels

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

External Links

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L233-L261

Vulnerability details

There is only exist 2 fighterType. 0 for premium fighter with custom DNA, 1 for default fighter.

FighterFarm.redeemMintPass() allow user input anything as long as they have enough mint pass to burn. This include FighterType id which can be any number.

Impact

FighterFarm._createNewFighter() does not fail when fighterType > 1. Mint will success, there will exist an NFT with invalid fighterType.

The only one might have problem with this is game client or gameserver handling battle simulation might not have any logic to handle non exist fighterType. Causing unknown problem.

Proof of Concept

Redeem have no input verification https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L233-L261

FighterType can only be 1 or 0. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L127

Fighter DNA will be fighterType id https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L462-L474

generation will return 0 and numsElement[] will always be 3. There is nothing to prevent minting fighterType > 1 from failing.

Tools Used

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L233-L261

Add require check for Fighter Type

    function _createNewFighter(
        address to, 
        uint256 dna, //@hash from msg.sender . can be manipulated
        string memory modelHash,//@user ipfs url
        string memory modelType, //@user "original"
        uint8 fighterType,//0 or 1 //@audit M redeem user can use non exist fighter type to bypass DNA _createFighterBase()
        uint8 iconsType,//0
        uint256[2] memory customAttributes//@[uint256(100), uint256(100)] default value or from merging pools
    ) 
        private 
    {  
        require(fighterType < 2);//@
        require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);

Assessed type

Context

#0 - c4-pre-sort

2024-02-22T08:07:48Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-22T08:07:56Z

raymondfam marked the issue as duplicate of #33

#2 - c4-pre-sort

2024-02-26T00:53:58Z

raymondfam marked the issue as duplicate of #1626

#3 - c4-judge

2024-03-05T10:56:27Z

HickupHH3 changed the severity to 3 (High Risk)

#4 - c4-judge

2024-03-06T03:35:57Z

HickupHH3 marked the issue as satisfactory

Lines of code

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

Vulnerability details

Impact

User of wrong fighterType can reroll beyond limit of maxRerollsAllowed[fighterType].

Proof of Concept

There is only 2 fighters type ID. /// @param fighterType Type of fighter either 0 or 1 (champion or dendroid).

Each type can reroll max 3 times.

/// @notice The maximum amount of rerolls for each fighter. uint8[2] public maxRerollsAllowed = [3, 3];

After admin increase generation, reroll limit increase by 1. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L125-L134

So if admin increase generation of champion to 10, while dendroid is 5. Champion can reroll 10 times. While dendroid can reroll 5 times.

Because reRoll() function does not check if token have correct fighterType as user input. Dendroid User can reroll() 10 times with this input fighterType = Champion despite being wrong type. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L370-L391

Tools Used

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

Change fighterType to enum.

    /// @notice Rolls a new fighter with random traits.
    /// @param tokenId ID of the fighter being re-rolled.
    /// @param fighterType The fighter type.
    function reRoll(uint8 tokenId, uint8 fighterType) public {
        require(msg.sender == ownerOf(tokenId));//@audit can reroll despite wrong fighter type
        require( (fighters[tokenId].dendroidBool?1:0) == (fighterType), "Fighter type does not match");
    }   

Assessed type

Other

#0 - c4-pre-sort

2024-02-22T02:32:11Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-22T02:32:18Z

raymondfam marked the issue as duplicate of #306

#2 - c4-judge

2024-03-05T04:36:41Z

HickupHH3 marked the issue as satisfactory

#3 - c4-judge

2024-03-19T09:05:07Z

HickupHH3 changed the severity to 3 (High Risk)

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L530-L532 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L519-L534 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L438-L439

Vulnerability details

10 user stake 1e1 (1wei) token earn the same amount total points as single user stake 100e18 $NRN. 100 user stake 1e1 (1wei) token earn the same amount total points as single user stake 10_000e18 $NRN.

Rewards distribution per round spread evenly for all player based on points. This leads to unfair rewards distribution.

Also because of rounding issue, user who deposit less than 1_000 token never get their stake removed when lost battle. Basically encourage user to stake 1 wei token for entire round.

They will get same amount of points on ranking same as user who put actual money on staking.

Impact

Rewards distribute evenly per round for all player based on points. Points can be earned by staking 1 wei token have same weight as staking 2e18 token.

Basically free points and rewards for doing absolutely nothing and not putting any serious money at risk.

Proof of Concept

Rewards distribution per round is based on points ranking. The more point you have, the more rewards you get.

Only user who have token staked allowed to earn points after battle

//https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L341-L344
        uint256 stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);//@stake always 0 after 1st stake
        if (amountStaked[tokenId] + stakeAtRisk > 0) {
            _addResultPoints(battleResult, tokenId, eloFactor, mergingPortion, fighterOwner);
        }//@audit user with no stake cannot add resultPoints. There is never points increase.

Look at stake function. There is no minimum stake required. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L241-L265

And how points earned is calculated based on StakingFactor and elo.

//https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L416-L500
        /// Check how many NRNs the fighter has at risk
        stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);
        /// Calculate the staking factor if it has not already been calculated for this round 
        if (_calculatedStakingFactor[tokenId][roundId] == false) {
            stakingFactor[tokenId] = _getStakingFactor(tokenId, stakeAtRisk);//@minimum 1 as staking factor
            _calculatedStakingFactor[tokenId][roundId] = true;//@ staking factor decimal is 0. if deposit 100e18. staking factor is 100^0.5=10
        }

        // .......

        /// If the user has no NRNs at risk, then they can earn points
        if (stakeAtRisk == 0) {
            points = stakingFactor[tokenId] * eloFactor;
        }

There is problem with StakingFactor calculation.

Deposit 100e18 token give you square root of 100=10 staking factor. Deposit 2e18 token give you square root of 2=1 staking factor. Deposit 1e1 token give you 0 staking factor and rounded up to 1.

It always rounding up to 1 when deposit staking token is less than 2e18.

    function _getStakingFactor(
        uint256 tokenId, 
        uint256 stakeAtRisk
    ) 
        private 
        view 
        returns (uint256) 
    {
      uint256 stakingFactor_ = FixedPointMathLib.sqrt(
          (amountStaked[tokenId] + stakeAtRisk) / 10**18//@audit R getstaking factor seem manipulated by staking <1e18
      );
      if (stakingFactor_ == 0) {
        stakingFactor_ = 1;//@ staking always round up to 1
      }
      return stakingFactor_;
    }

This mean user deposit 1e1 token have same staking factor as user deposit 1e18 token. They allowed to earn points and rewards based on points earned.

Also due to another rounding issue.

    /// Potential amount of NRNs to put at risk or retrieve from the stake-at-risk contract
    curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;//@div  10000 //@audit H no minimum staking amount result in never get stake at risk

User who deposit less than 1_000 token never get their stake at risk due to curStakeAtRisk == 0. This in turn prevent contract from remove their stake when they lose battle. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L491-L498

Combined all previous issues

This incentived cheating user to deposit 1 wei every round to earn points and some token rewards. It cost nothing and still earn rewards. As automatic battle and game server will keep sending battle report to update for all users.

Tools Used

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L519-L534

Add minimum stake required to earn points and rewards.

    uint256 public minimumDeposit = 10e18;
    function setMinimumDeposit(uint256 _minimumDeposit) external onlyOwner {
        minimumDeposit = _minimumDeposit;
    }

    function stakeNRN(uint256 amount, uint256 tokenId) external {//@audit can stake for ongoing round. This mean when battle going for halfway. User can still stake for winner?
        require(amount > minimumDeposit, "Amount cannot less than minimumDeposit");

Assessed type

Math

#0 - c4-pre-sort

2024-02-22T17:12:26Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-22T17:13:09Z

raymondfam marked the issue as duplicate of #38

#2 - c4-judge

2024-03-07T02:49:49Z

HickupHH3 changed the severity to 2 (Med Risk)

#3 - c4-judge

2024-03-07T02:58:22Z

HickupHH3 changed the severity to 3 (High Risk)

#4 - c4-judge

2024-03-07T03:37:42Z

HickupHH3 marked the issue as satisfactory

Lines of code

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

Vulnerability details

Array numElements[] is init once in FighterFarm.sol constructor. But there is no function to update it.

Causing dna % numElements[generation[fighterType]]; to always revert later when generation is increase by 1. numElements will always return 0. Modular operation % 0 always revert.

Impact

Math division by 0. It will always revert when minting new NFT for non-zero generation.

Proof of Concept

Inside FighterFarm.sol constructor, numElements is init once only for generation 0. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L104-L111

Admin can increase generation later. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L129-L134

When generation[fighterType] > 0. numElements[1] will return 0.

and this will revert. uint256 element = dna % numElements[generation[fighterType]];

NFT minting calling _createFighterBase() will just fail. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L499-L501 Prevent further NFT minting

Tools Used

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

    /// @notice Increase the generation of the specified fighter type.
    /// @dev Only the owner address is authorized to call this function.
    /// @param fighterType Type of fighter either 0 or 1 (champion or dendroid).
    /// @return Generation count of the fighter type.
    function incrementGeneration(uint8 fighterType, uint newNumElements) external returns (uint8) {
        require(msg.sender == _ownerAddress);
        generation[fighterType] += 1;//@audit H admin increase generation of fighter type will destroy minting new fighter of that type due to element not exist for new generation
        maxRerollsAllowed[fighterType] += 1;
+        numElements[generation[fighterType]] = newNumElements;
        return generation[fighterType];
    }

Assessed type

Math

#0 - c4-pre-sort

2024-02-22T19:06:40Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-22T19:06:48Z

raymondfam marked the issue as duplicate of #45

#2 - c4-judge

2024-03-08T03:18:49Z

HickupHH3 marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L292-L311 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/MergingPool.sol#L139-L167

Vulnerability details

RankedBattle.claimNRN() and MergingPool.claimRewards() code are gas inefficient that loop through all rounds to find rewards to claim.

For L2 game, if the game start new round every 1 hour. After 6000 rounds, or 245 days, new user claiming rewards transaction will reach 30M block gas limit.

Impact

After 6000 rounds, no new user will be able to claim rewards anymore. Required gas to succeed transaction will be higher than block gas limit.

Proof of Concept

Because after user stake token. Stake will carry on to new rounds. User can passively earn points if someone battle them. So looping through every single round to check if user have rewards that round make sense

But it extremely really inefficient logic.

This loop run every time new user claim rewards. This mean if current round is 500. New user have to loop through 500 previous rounds to check if they have any reward.

Running below foundry test to simulate this. After 6000 rounds, it cost user 30M gas to just call claimNRN() the first time. Same things with MergingPool.claimRewards(). Same logic, same problem.

Modify some file to run these test.

test\RankedBattle.t.sol
        /// @notice Test 2 accounts staking, winning a battle, setting a new round and claiming NRN for the previous round.
    function testClaimNRNGas() public {
        //Set round to 4000
        _rankedBattleContract.debugSetNewRound(6000);
        address staker = vm.addr(3);
        //print gas report
        uint gasLeft = gasleft();
        vm.prank(staker);        
        _rankedBattleContract.claimNRN();
        console.log("Gas used: ", gasLeft - gasleft());
    }
src\RankedBattle.sol
    function debugSetNewRound(uint setRound) external {
        require(isAdmin[msg.sender]);
        roundId = setRound;
        _stakeAtRiskInstance.setNewRound(roundId);//@audit-ok M global staked does not update when stake at risk is removed
        rankedNrnDistribution[roundId] = rankedNrnDistribution[roundId - 1];
    }
    //@Edit
    function claimNRN() external {
        require(numRoundsClaimed[msg.sender] < roundId, "Already claimed NRNs for this period");//@audit M out of gas claimNRN after 4000 rounds. Tested through foundry
        uint256 claimableNRN = 0;
        uint256 nrnDistribution;
        uint32 lowerBound = numRoundsClaimed[msg.sender];
        for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
            nrnDistribution = getNrnDistribution(currentRound);
            if(totalAccumulatedPoints[currentRound]>0)
                claimableNRN += (
                    accumulatedPointsPerAddress[msg.sender][currentRound] * nrnDistribution   
                ) / totalAccumulatedPoints[currentRound];
            else claimableNRN += 1; //@for debug gas
            numRoundsClaimed[msg.sender] += 1;
        }
        if (claimableNRN > 0) {
            amountClaimed[msg.sender] += claimableNRN;
            _neuronInstance.mint(msg.sender, claimableNRN);
            emit Claimed(msg.sender, claimableNRN);
        }
    }

Final result

Running 1 test for test/RankedBattle.t.sol:RankedBattleTest [PASS] testClaimNRNGas() (gas: 31636965) Logs: Gas used: 31550102

Tools Used

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L292-L311 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/MergingPool.sol#L139-L167

Limit loop to 1000 per transaction. Or force user claimNRN every time stake or unstake.

    /// @notice Claims NRN tokens for the specified rounds.
    /// @dev Caller can only claim once per round.
    function claimNRN() external {
        require(numRoundsClaimed[msg.sender] < roundId, "Already claimed NRNs for this period");//@audit M out of gas claimNRN after 4000 rounds. Tested through foundry
        uint256 claimableNRN = 0;
        uint256 nrnDistribution;
        uint32 lowerBound = numRoundsClaimed[msg.sender];
+        uint roundLoop = math.Max(roundId, lowerBound + 1000);
+        for (uint32 currentRound = lowerBound; currentRound < roundLoop; currentRound++) {
            nrnDistribution = getNrnDistribution(currentRound);
            claimableNRN += (
                accumulatedPointsPerAddress[msg.sender][currentRound] * nrnDistribution   
            ) / totalAccumulatedPoints[currentRound];
        }
+        numRoundsClaimed[msg.sender] += roundLoop;
        if (claimableNRN > 0) {
            amountClaimed[msg.sender] += claimableNRN;
            _neuronInstance.mint(msg.sender, claimableNRN);
            emit Claimed(msg.sender, claimableNRN);
        }
    }

Assessed type

DoS

#0 - c4-pre-sort

2024-02-24T00:03:38Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-24T00:04:22Z

raymondfam marked the issue as duplicate of #1541

#2 - c4-judge

2024-03-11T13:01:33Z

HickupHH3 marked the issue as duplicate of #216

#3 - c4-judge

2024-03-11T13:08:05Z

HickupHH3 changed the severity to 2 (Med Risk)

#4 - c4-judge

2024-03-12T02:36:20Z

HickupHH3 marked the issue as partial-50

#5 - c4-judge

2024-03-21T02:58:59Z

HickupHH3 marked the issue as satisfactory

Lines of code

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

Vulnerability details

FighterFarm.reRoll() take fee and provide income for project. Any DNA manipulation to prevent further reroll is an issue.

Impact

NFT DNA reroll can be manipulated to get maximum attribute and rarity. Basically cheating to get best NFT for battle simulation

Proof of Concept

reroll DNA using msg.sender,tokenId,numRerolls as hash uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));

All 3 components is predictable. NFT can be transfer away. So msg.sender can be anything that user generate.

When user can generate their own DNA. Look at how rarity is generated. https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/AiArenaHelper.sol#L98-L119

They only need to find DNA that rarityPoint = (dna / [2,3,5,11,13] ) % 100 and still give high rarity. Give user best physical attributes for their NFT

Tools Used

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

Use tokenId and numRerolls only if do not want user to control reroll process. uint256 dna = uint256(keccak256(abi.encode(tokenId, numRerolls[tokenId])));

Assessed type

Math

#0 - c4-pre-sort

2024-02-24T01:54:07Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-02-24T01:54:22Z

raymondfam marked the issue as duplicate of #53

#2 - c4-judge

2024-03-06T03:49:26Z

HickupHH3 changed the severity to 3 (High Risk)

#3 - c4-judge

2024-03-06T03:52:22Z

HickupHH3 marked the issue as satisfactory

#4 - c4-judge

2024-03-15T02:10:54Z

HickupHH3 changed the severity to 2 (Med Risk)

#5 - c4-judge

2024-03-22T04:21:48Z

HickupHH3 marked the issue as duplicate of #376

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L493-L497 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L539-L545

Vulnerability details

FighterFarm.sol NFT suppose to be transferable to another account if have no staked token. But is not due to missing logic check in RankedBattle._addResultPoints().

Impact

NFT token no longer have staked token. But still cannot transfer to another account. It require user to call unstake() zero token is make it transferable

Proof of Concept

NFT cannot be transferred to another account if it is staked.

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

Stake NFT can only happen in RankedBattle.sol https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L244-L290

Look at this logic. When token no longer have any stake, remove staking status

        if (amountStaked[tokenId] == 0) {
            _fighterFarmInstance.updateFighterStaking(tokenId, false);
        }

RankedBattle.updateBattleRecord() can remove all amountStaked[tokenId] when fighter lost battle. These stake token are moved to stake at risk. But does not update staking status when it no longer have any stake.


    bool success = _neuronInstance.transfer(_stakeAtRiskAddress, curStakeAtRisk);
    if (success) {
        _stakeAtRiskInstance.updateAtRiskRecords(curStakeAtRisk, tokenId, fighterOwner);
        amountStaked[tokenId] -= curStakeAtRisk;
        //@curStakeAtRisk is clamped max to amountStaked[tokenId]
        //@fund is moved to stake at risk
    }

When round is over, admin can sweep all stake at risk to treasury. Then token will no longer have any stake but staking status still persist. This prevent NFT from transfer away unless call unstake() first.

Tools Used

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L493-L497 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L539-L545

Always update staking status.

        /// If the user has stake-at-risk for their fighter, reclaim a portion
        /// Reclaiming stake-at-risk puts the NRN back into their staking pool
        if (curStakeAtRisk > 0) {
+            if(amountStaked[tokenId] == 0) {
+                _fighterFarmInstance.updateFighterStaking(tokenId, true);
+            }
            _stakeAtRiskInstance.reclaimNRN(curStakeAtRisk, tokenId, fighterOwner);
            amountStaked[tokenId] += curStakeAtRisk;
        }

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

Assessed type

Token-Transfer

#0 - c4-pre-sort

2024-02-24T05:06:29Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-24T05:06:36Z

raymondfam marked the issue as primary issue

#2 - raymondfam

2024-02-24T05:07:04Z

Inconsistent and adequate proof when compared to #1641.

#3 - c4-judge

2024-03-13T10:11:36Z

HickupHH3 marked the issue as partial-75

#4 - c4-judge

2024-03-13T10:11:56Z

HickupHH3 marked the issue as duplicate of #1641

Awards

59.2337 USDC - $59.23

Labels

bug
2 (Med Risk)
insufficient quality report
satisfactory
duplicate-43

External Links

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L345-L347 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L268-L276 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L253-L255 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L539-L545

Vulnerability details

User NFT fighter need 10 energy for one battle. The daily limit 100 energy. Then user need to pay 10_000 NRN for battery game item to recharge all energy.

Because NFT FighterFarm.sol only prevent transfer in case of NFT already have NRN staked. And RankedBattle.sol allow user to fight with no money at stake.

Basically, user can transfer NFT to another address after 10 rounds and energy will be restore to full as energy storage unique to owner address not token.

Impact

Loss of income from battery sale for developer.

User with no stake can have infinite battle. If they decided to only attack user with staked NFT and they won. User with staked NFT will lose their stake despite battle count limitation through energy spending check.

Proof of Concept

  1. NFT transfer only check if token is staked
    function _ableToTransfer(uint256 tokenId, address to) private view returns(bool) {
        return (
          _isApprovedOrOwner(msg.sender, tokenId) &&
          balanceOf(to) < MAX_FIGHTERS_ALLOWED &&
          !fighterStaked[tokenId]
        );
    }
  1. Staked status is only updated by RankedBattle
    function updateFighterStaking(uint256 tokenId, bool stakingStatus) external {
        require(hasStakerRole[msg.sender]);
        fighterStaked[tokenId] = stakingStatus;
        if (stakingStatus) {
            emit Locked(tokenId);
        } else {
            emit Unlocked(tokenId);
        }
    }
  1. RankedBattle only lock NFT transfer after stake token to NFT. Link
  2. Battle results are updated only by offchain bot/gameServer. User can battle with no stake-at-risk. Link
  3. VoltageManager refresh energy per address every 24 hours. Link

Exploiting VoltageManager energy refresh for all new address. By transferring NFT away after energy is depleted. Energy will complete recharge. So user with no stake can freely battle without spending money buying battery pack to recharge.

It cost 10000 NRN token to buy battery. So user can avoid paying to play more than 10 battles per day.

Tools Used

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L345-L347 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L268-L276 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/RankedBattle.sol#L253-L255 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L539-L545

It make more sense to attach energy requirement spending/recharge to each NFT token instead of owner address.

Easiest fix, make NFT transfer also cost 1 energy voltage.

Assessed type

Token-Transfer

#0 - c4-pre-sort

2024-02-25T18:00:40Z

raymondfam marked the issue as insufficient quality report

#1 - c4-pre-sort

2024-02-25T18:00:59Z

raymondfam marked the issue as duplicate of #43

#2 - c4-judge

2024-03-07T06:26:59Z

HickupHH3 marked the issue as satisfactory

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