Fighter NFT with no stake-at-risk can bypass daily Energy recharge requirement without paying for battery. By transferring NFT to another address after battle

[c4-bot-10]

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

2. 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);
        }
    }

3. RankedBattle only lock NFT transfer after stake token to NFT. Link
4. Battle results are updated only by offchain bot/gameServer. User can battle with no stake-at-risk. Link
5. 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

Recommended Mitigation Steps

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

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #43

[c4-judge]

HickupHH3 marked the issue as satisfactory