search

code-423n4 / 2024-02-ai-arena-findings

4 stars 3 forks source link

getStakeAtRisk function always returns the stake amount instead of the expected stake at risk amount. #584

Closed c4-bot-1 closed 7 months ago

c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/StakeAtRisk.sol#L132-L134

Vulnerability details

Impact

Users relying on the getStakeAtRisk function may receive inaccurate information about the stake at risk for a fighter in the current round.

Proof of Concept

In the StakeAtRisk contract, the getStakeAtRisk function retrieves the stake amount for a fighter in the current round directly from the stakeAtRisk mapping without considering any reclaimed stakes. As a result, the function consistently returns the total stake amount without adjusting for stakes that have been reclaimed by the fighter. This oversight leads to an incorrect representation of the stake at risk, impacting the reliability and fairness of the system.

function getStakeAtRisk(uint256 fighterId) external view returns(uint256) {
    return stakeAtRisk[roundId][fighterId];
}

Test Case and Test Result: Test Case:

/// @notice Test getting the stake at risk for a players fighter that has stake at risk.
    function testGetStakeAtRisk() public {
        address player = vm.addr(3);
        uint256 stakeAmount = 3_000 * 10 ** 18;
        uint256 expectedStakeAtRiskAmount = (stakeAmount * 100) / 100000;
        uint256 tokenId = 0;

        // player gets fighter
        _mintFromMergingPool(player);
        assertEq(_fighterFarmContract.ownerOf(tokenId), player);

        // player gets NRN
        _fundUserWith4kNeuronByTreasury(player);
        vm.prank(player);
        // player stakes NRN
        _rankedBattleContract.stakeNRN(stakeAmount, 0);
        assertEq(_rankedBattleContract.amountStaked(0), stakeAmount);

        // player battles
        vm.prank(address(_GAME_SERVER_ADDRESS));
        // loses battle
        _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
        assertEq(_stakeAtRiskContract.stakeAtRisk(0, 0), expectedStakeAtRiskAmount);
        assertEq(_stakeAtRiskContract.getStakeAtRisk(tokenId), expectedStakeAtRiskAmount);
    }

Test Result:

[⠢] Compiling...
No files changed, compilation skipped

Running 1 test for test/StakeAtRisk.t.sol:StakeAtRiskTest
[PASS] testGetStakeAtRisk() (gas: 879916)
Traces:
  [879916] StakeAtRiskTest::testGetStakeAtRisk()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69
    ├─ [0] VM::prank(MergingPool: [0xBB807F76CdA53b1b4256E1b6F33bB46bE36508e3])
    │   └─ ← ()
    ├─ [432889] FighterFarm::mintFromMergingPool(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, "_neuralNetHash", "original", [1, 80])
    │   ├─ [85155] AiArenaHelper::createPhysicalAttributes(108450779827070475631476400247280731650835146888089526492857736153573717308856 [1.084e77], 0, 0, false) [staticcall]
    │   │   └─ ← FighterPhysicalAttributes({ head: 2, eyes: 5, mouth: 4, body: 2, hands: 2, feet: 4 })
    │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, tokenId: 0)
    │   ├─ [2260] FighterOps::d8d02d30(0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000) [delegatecall]
    │   │   ├─ emit FighterCreated(id: 0, weight: 80, element: 1, generation: 0)
    │   │   └─ ← ()
    │   └─ ← ()
    ├─ [581] FighterFarm::ownerOf(0) [staticcall]
    │   └─ ← 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69
    ├─ [0] VM::prank(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf)
    │   └─ ← ()
    ├─ [29938] Neuron::transfer(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, 4000000000000000000000 [4e21])
    │   ├─ emit Transfer(from: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf, to: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, value: 4000000000000000000000 [4e21])
    │   └─ ← true
    ├─ [586] Neuron::balanceOf(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69) [staticcall]
    │   └─ ← 4000000000000000000000 [4e21]
    ├─ [0] VM::prank(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69)
    │   └─ ← ()
    ├─ [194200] RankedBattle::stakeNRN(3000000000000000000000 [3e21], 0)
    │   ├─ [581] FighterFarm::ownerOf(0) [staticcall]
    │   │   └─ ← 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69
    │   ├─ [586] Neuron::balanceOf(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69) [staticcall]
    │   │   └─ ← 4000000000000000000000 [4e21]
    │   ├─ [26991] Neuron::approveStaker(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, RankedBattle: [0x1B5d22530787C7C8c27021103d7C63F6198781ba], 3000000000000000000000 [3e21])
    │   │   ├─ emit Approval(owner: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, spender: RankedBattle: [0x1B5d22530787C7C8c27021103d7C63F6198781ba], value: 3000000000000000000000 [3e21])
    │   │   └─ ← ()
    │   ├─ [27848] Neuron::transferFrom(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, RankedBattle: [0x1B5d22530787C7C8c27021103d7C63F6198781ba], 3000000000000000000000 [3e21])
    │   │   ├─ emit Approval(owner: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, spender: RankedBattle: [0x1B5d22530787C7C8c27021103d7C63F6198781ba], value: 0)
    │   │   ├─ emit Transfer(from: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, to: RankedBattle: [0x1B5d22530787C7C8c27021103d7C63F6198781ba], value: 3000000000000000000000 [3e21])
    │   │   └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001
    │   ├─ [25859] FighterFarm::updateFighterStaking(0, true)
    │   │   ├─ emit Locked(tokenId: 0)
    │   │   └─ ← ()
    │   ├─ [4618] StakeAtRisk::getStakeAtRisk(0) [staticcall]
    │   │   └─ ← 0
    │   ├─ emit Staked(from: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, amount: 3000000000000000000000 [3e21])
    │   └─ ← ()
    ├─ [471] RankedBattle::amountStaked(0) [staticcall]
    │   └─ ← 3000000000000000000000 [3e21]
    ├─ [0] VM::prank(0x7C0a2BAd62C664076eFE14b7f2d90BF6Fd3a6F6C)
    │   └─ ← ()
    ├─ [210066] RankedBattle::updateBattleRecord(0, 50, 2, 1500, true)
    │   ├─ [581] FighterFarm::ownerOf(0) [staticcall]
    │   │   └─ ← 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69
    │   ├─ [2600] VoltageManager::ownerVoltageReplenishTime(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69) [staticcall]
    │   │   └─ ← 0
    │   ├─ [618] StakeAtRisk::getStakeAtRisk(0) [staticcall]
    │   │   └─ ← 0
    │   ├─ [618] StakeAtRisk::getStakeAtRisk(0) [staticcall]
    │   │   └─ ← 0
    │   ├─ [25138] Neuron::transfer(StakeAtRisk: [0x349391A6596ACF133B947BB4544C329C217c227e], 3000000000000000000 [3e18])
    │   │   ├─ emit Transfer(from: RankedBattle: [0x1B5d22530787C7C8c27021103d7C63F6198781ba], to: StakeAtRisk: [0x349391A6596ACF133B947BB4544C329C217c227e], value: 3000000000000000000 [3e18])
    │   │   └─ ← true
    │   ├─ [68957] StakeAtRisk::updateAtRiskRecords(3000000000000000000 [3e18], 0, 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69)
    │   │   ├─ emit IncreasedStakeAtRisk(fighterId: 0, atRiskAmount: 3000000000000000000 [3e18])
    │   │   └─ ← ()
    │   ├─ [47513] VoltageManager::spendVoltage(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, 10)
    │   │   ├─ emit VoltageRemaining(spender: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, voltage: 90)
    │   │   └─ ← ()
    │   └─ ← ()
    ├─ [545] StakeAtRisk::stakeAtRisk(0, 0) [staticcall]
    │   └─ ← 3000000000000000000 [3e18]
    ├─ [618] StakeAtRisk::getStakeAtRisk(0) [staticcall]
    │   └─ ← 3000000000000000000 [3e18]
    └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.82ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual. Foundry

Recommended Mitigation Steps

The getStakeAtRisk function should be modified to deduct any reclaimed stakes from the total stake at risk for the fighter in the current round before returning the result.

Assessed type

Other

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

raymondfam commented 8 months ago

reclaimNRN() has it well taken care of.

c4-pre-sort commented 8 months ago

raymondfam marked the issue as primary issue

c4-judge commented 7 months ago

HickupHH3 marked the issue as unsatisfactory: Invalid