code-423n4 / 2024-03-abracadabra-money-findings

9 stars 7 forks source link

Oracle trust #233

Closed c4-bot-8 closed 7 months ago

c4-bot-8 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/oracles/aggregators/MagicLpAggregator.sol#L38

Vulnerability details

Impact

Detailed description of the impact of this finding. Oracle trust: Dependence on external oracles for price without checks. Type casting: uint256(baseOracle.latestAnswer()) might be negative.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

function latestAnswer() public view override returns (int256) { @> uint256 baseAnswerNomalized = uint256(baseOracle.latestAnswer()) * (10 * (WAD - baseOracle.decimals())); @> uint256 quoteAnswerNormalized = uint256(quoteOracle.latestAnswer()) (10 ** (WAD - quoteOracle.decimals())); uint256 minAnswer = baseAnswerNomalized < quoteAnswerNormalized ? baseAnswerNomalized : quoteAnswerNormalized;

    (uint256 baseReserve, uint256 quoteReserve) = _getReserves();
    baseReserve = baseReserve * (10 ** (WAD - baseDecimals));
    quoteReserve = quoteReserve * (10 ** (WAD - quoteDecimals));
    return int256(minAnswer * (baseReserve + quoteReserve) / pair.totalSupply());
}

Tools Used

Recommended Mitigation Steps

we should verify the baseOracle.latestAnswer() >0; we should verify the quoteOracle.latestAnswer()>0; uint256 baseAnswerNomalized = uint256(baseOracle.latestAnswer()) * (10 ** (WAD - baseOracle.decimals()));

uint256 quoteAnswerNormalized = uint256(quoteOracle.latestAnswer()) * (10 ** (WAD - quoteOracle.decimals()));

Assessed type

Oracle

0xm3rlin commented 8 months ago

As intended

141345 commented 8 months ago

verify price feed
no details about impact

QA is more appropriate

c4-pre-sort commented 8 months ago

141345 marked the issue as sufficient quality report

c4-sponsor commented 7 months ago

0xCalibur (sponsor) disputed

c4-judge commented 7 months ago

thereksfour changed the severity to QA (Quality Assurance)