AI Arena - lanrebayode77's results

Lines of code

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

Vulnerability details

Impact

The FighterFarm.reRoll() allows users to change attributes of their fighter like; element, weight, and physicalAttributes. The only constraint with this function call is that numRerolls[tokenId] < maxRerollsAllowed[fighterType].

However, even with the present constraint, user can still reroll fighter beyond the actual maxRerollsAllowed of the original fighterType

Proof of Concept

Originl Fighter type might be 0 or 1; (champion or dendroid). Initially, maxRerollsAllowed = [3, 3]

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

But any of the generations could be changed and maxRerollsAllowed will increment.

    function incrementGeneration(uint8 fighterType) external returns (uint8) {
        ...
        maxRerollsAllowed[fighterType] += 1;
        ...
    }

If dendroid generation was changed and maxRerollsAllowed[dendroid] increased to 4, a user could claim the fighter type to be dendroid when it is champion originally, so when the numRerolls[tokenId] = 3, the check will pass as it checks maxRerollsAllowed[dendroid] which is now 4.

 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");
...

Tools Used

Manual review.

Recommended Mitigation Steps

Validate that the fighter type is originally what the user claims it to be.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

When a player losses a battle with a staked fighter, _addResultPoints checks if the fighter has points, else some of the amountStaked[tokenId] is sent to _stakeAtRiskAddress . The user needs to win battles to recover stakeAtRisk. If the user stakeAtRisk is not recovered before the end of the round, it will be sent to treasury.

However, a player can avoid losing stakeAtRisk to treasury, recover them all, and accrue points with zero stakedAmounts

Proof of Concept

1. Alice stakes 1,000,000 NRM on a fighter.
2. Alice lost the first battle, with no point, and part of the stake is sent to _stakeAtRiskAddress.

 /// Potential amount of NRNs to put at risk or retrieve from the stake-at-risk contract
        curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 10**4;

From the snippet above, Alice curStakeAtRisk = 1e21 1_000 NRN tokens. 3. After this result,stakeAtRisk = 1000

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

Alice unstakes the rest(the only implication to this is the inability to stake in the same round, but the player will still participate in battles). 4. After unstaking the entire amount, the next battle result was another loss, which means Alice stakeAtRisk ought to increase since there is no points to deduct from, but curStakeAtRisk has been turned to zero due to a check;

 /// Do not allow users to lose more NRNs than they have in their staking pool
            if (curStakeAtRisk > amountStaked[tokenId]) {
                curStakeAtRisk = amountStaked[tokenId]; //@audit including zero?
            }

Since amountStaked[tokenId] is now zero, curStakeAtRisk = (10 * (0+1000e18))/10e4 = 4e18, curStakeAtRisk is then set to zero.

 /// If the fighter does not have any points for this round, NRNs become at risk of being lost
                bool success = _neuronInstance.transfer(_stakeAtRiskAddress, curStakeAtRisk);

from the snippet above, zero will be transferred to _stakeAtRiskAddress and only zero is deducted from amountStaked[tokenId], so nothing will revert, since both values are zero.

Even though Alice cannot have more stake put at risk, if a battle is won part of the stake at risk will be recovered. here

            /// 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) {
                _stakeAtRiskInstance.reclaimNRN(curStakeAtRisk, tokenId, fighterOwner);
                amountStaked[tokenId] += curStakeAtRisk;
            }

If the Alice can fully recover the stake at risk, points can begin to accumulate and can be used to claim rewards.

Paste the test in RankedBattle.t.sol

    function test_AvoidLosingStakeAndRecoverLostStake() public {
        address player1 = vm.addr(3);
        _mintFromMergingPool(player1);
        vm.prank(_treasuryAddress);
        _neuronContract.transfer(player1, 1_000_000 * 10 ** 18);
        vm.prank(player1);
        _rankedBattleContract.stakeNRN(1_000_000 * 10 ** 18, 0);
        assertEq(_rankedBattleContract.amountStaked(0), 1_000_000 * 10 ** 18);

        //After the Battle, Update is done
        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        uint256 StakeFactor_Player1 = _rankedBattleContract.stakingFactor(0);
        console.log(
            "==============================================================================="
        );
        console.log("s.F Player1", StakeFactor_Player1);
        uint256 player1_Point = _rankedBattleContract
            .accumulatedPointsPerFighter(0, 0);
        console.log("Player1 POINT:", player1_Point);
        uint256 initial_StakeAtRisk_Player1 = _stakeAtRiskContract
            .getStakeAtRisk(0);
        console.log("Player1 Stake at Risk:", initial_StakeAtRisk_Player1);
        console.log(
            "==============================================================================="
        );

        //Player1 enter a battle and saw that this is a loss, then frontruns the updateBattleRecord() ustake leaving 1wei
        vm.prank(player1);
        _rankedBattleContract.unstakeNRN(1_000_000 * 10 ** 18, 0); //Loaned/flashloan/personal funds
        assertEq(_rankedBattleContract.amountStaked(0), 0);

        vm.prank(address(_GAME_SERVER_ADDRESS));
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        console.log(
            "Player1 Stake at Risk After the second lost:",
            _stakeAtRiskContract.getStakeAtRisk(0)
        );
        console.log(
            "Amount STAKED after Loss and Unstake:",
            _rankedBattleContract.amountStaked(0)
        );
        console.log(
            "==============================================================================="
        );

        for (uint256 i = 0; i < 8; i++) {
            vm.prank(address(_GAME_SERVER_ADDRESS));
            _rankedBattleContract.updateBattleRecord(0, 50, 0, 1500, true);
        }

        console.log(
            "Player1 Stake at Risk After 8 Wins:",
            _stakeAtRiskContract.getStakeAtRisk(0)
        );
        console.log(
            "Amount Recovered after 8 wins:",
            initial_StakeAtRisk_Player1 - _stakeAtRiskContract.getStakeAtRisk(0)
        );
        console.log(
            "Amount STAKED Now:",
            _rankedBattleContract.amountStaked(0)
        );
        console.log(
            "==============================================================================="
        );
    }

Logs:
  ===============================================================================
  s.F Player1 1000
  Player1 POINT: 0
  Player1 Stake at Risk: 1000000000000000000000
  ===============================================================================
  Player1 Stake at Risk After the second lost: 1000000000000000000000
  Amount STAKED after Loss and Unstake: 0
  ===============================================================================
  Player1 Stake at Risk After 8 Wins: 992000000000000000000
  Amount Recovered after 8 wins: 8000000000000000000
  Amount STAKED Now: 8000000000000000000
  ===============================================================================

Tools Used

Manual review and foundry

Recommended Mitigation Steps

Since, curStakeAtRisk will be set to zero if no stakedAmount, amount to be recovered must be the least balance of amountStaked[tokenId], else all stake at risk should be sent to treasury after the round, to avoid players using zero stke to recover stake at risk gradually and having nothing to loss doing so.

 /// Do not allow users to lose more NRNs than they have in their staking pool
            if (curStakeAtRisk > amountStaked[tokenId]) {
                curStakeAtRisk = amountStaked[tokenId]; //@audit including zero?
            }

            /// 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) {
           //@audit stake must be equal/greater than recovery 
          +++    require(amountStaked[tokenId] >= curStakeAtRisk);
                _stakeAtRiskInstance.reclaimNRN(curStakeAtRisk, tokenId, fighterOwner);
                amountStaked[tokenId] += curStakeAtRisk;
            }

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

Within the GameItems contract, administrators possess the authority to generate game items and establish their prices. Furthermore, administrators dictate whether a game item is finite or infinite. To regulate the fair and balanced usage of game items, administrators also define the daily allowance for each item. If a player surpasses this daily limit, they must wait until the subsequent 24-hour period to make additional purchases.

However, the capability to transfer game items between addresses presents a challenge. Currently, there is no validation in place to ensure that the recipient address remains within the daily allowance limit set by the administrator. This oversight allows an address to acquire more than the prescribed daily allowance within a single day.

Moreover, this lack of validation carries additional implications, particularly concerning items with limited availability (finite). In scenarios where such items possess extraordinary attributes, resourceful users may purchase the entire supply and use them to climb leaderboard easily. This issue is particularly pronounced when these rare items confer significant advantages, such as protective shields or super gloves, effectively guaranteeing victory in battles.

Proof of Concept

1. An admin creates a game item(super glove) with limited supply(1,000), priced at 10_$NRN each, while the daily allowance is set to 10.
2. Alice decides to credit 100 address with $NRN_100 and buy it up under 10days to be transferred to Alice address.
3. Alice can now use this super item to advantage and win battles and gain more points for rewards which can be used to cover her expensed and make profit.

    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable);
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

Tools Used

Manual review

Recommended Mitigation Steps

    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable);
   +++  require(amount + balanceOf(to, tokenId) <= allowanceRemaining[msg.sender][tokenId], "Daily limit exceeded for the receiver");
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

Assessed type

Invalid Validation