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

4 stars 3 forks source link

Unchecked `eloFactor` allows arbitrary point manipulation, risking competition integrity. #1433

Closed c4-bot-3 closed 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L322-L349 https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L416-L424

Vulnerability details

Impact

No validation on the eloFactor passed back on battle outcomes. Could magnify points arbitrarily. In the updateBattleRecord() function, the opaque eloFactor gets passed. There are no checks on the value of eloFactor before it directly influences final rewards.

Since eloFactor directly scales reward points, a malicious game server could:

Without validation, the game servers have full control to arbitrarily tune rewards distribution. This subverts the competitive integrity underpinning the game economy.

Proof of Concept

To exploit, the game server would pass unrealistic values for eloFactor, whether +1000x or negative numbers. This then flows down into the private _distributeRewards() function to disproportionately impact final points used for rewards.

File::src/RankedBattle.sol::updateBattleRecord File::src/RankedBattle.sol::_addResultPoints

function updateBattleRecord(
    uint256 tokenId, 
    uint256 mergingPortion,
    uint8 battleResult,
    uint256 eloFactor, // ⚠️ Vulnerable input
    bool initiatorBool
) 
    external 
{   
    require(msg.sender == _gameServerAddress);
    require(mergingPortion <= 100);
    address fighterOwner = _fighterFarmInstance.ownerOf(tokenId);
    require(
        !initiatorBool ||
        _voltageManagerInstance.ownerVoltageReplenishTime(fighterOwner) <= block.timestamp || 
        _voltageManagerInstance.ownerVoltage(fighterOwner) >= VOLTAGE_COST
    );

    _updateRecord(tokenId, battleResult);
    uint256 stakeAtRisk = _stakeAtRiskInstance.getStakeAtRisk(tokenId);
    if (amountStaked[tokenId] + stakeAtRisk > 0) {
        _addResultPoints(battleResult, tokenId, eloFactor,// ⚠️ Passed here without validation
         mergingPortion, fighterOwner);
    }
    if (initiatorBool) {
        _voltageManagerInstance.spendVoltage(fighterOwner, VOLTAGE_COST);
    }
    totalBattles += 1;
}

function _addResultPoints(
    uint8 battleResult, 
    uint256 tokenId, 
    uint256 eloFactor,  // ⚠️ Consumed here
    uint256 mergingPortion,
    address fighterOwner
) 
    private 
{

    // Distributes rewards based on unvalidated eloFactor

}

The specific eloFactor parameter in the updateBattleRecord function definition, and showing precisely where that value gets propagated and consumed.

Tools Used

Vscode

Recommended Mitigation Steps

Not validating this critical input violates software security best practices and enables distortion of the core competitive game logic.

Assessed type

Invalid Validation

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #1155

c4-judge commented 8 months ago

HickupHH3 marked the issue as unsatisfactory: Invalid