min and maxAnswer never checked for oracle price feed

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-05-bakerfi/blob/main/contracts/oracles/EthOracle.sol#L31

Vulnerability details

Description

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. The result is that if an asset experiences a huge drop in value (i.e. LUNA crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset. This would allow user to continue borrowing with the asset but at the wrong price. This is exactly what happened to Venus on BSC when LUNA imploded. However, the protocol misses to implement such a check.

Link to code:

    function getLatestPrice() public view override returns (IOracle.Price memory price) {
@-->    (, int256 answer, uint256 startedAt, uint256 updatedAt,) = _ethPriceFeed.latestRoundData();
        if ( answer<= 0 ) revert InvalidPriceFromOracle();        
        if ( startedAt ==0 || updatedAt == 0 ) revert InvalidPriceUpdatedAt();    

        price.price = uint256(answer);
        price.lastUpdate = updatedAt;
    }

Similar past issues

• RaymondFam - Risk of Incorrect Asset Pricing by StableOracle in Case of Underlying Aggregator Reaching minAnswer sherlock-audit/2023-05-USSD-judging#598
• 0x52 - ChainlinkAdapterOracle will return the wrong price for asset if underlying aggregator hits minAnswer sherlock-audit/2023-02-blueberry-judging#18

Tools Used

Manual review

Recommended Mitigation Steps

Add logic along the lines of:

    require(answer >= minPrice && answer <= maxPrice, "invalid price");

min and max prices can be gathered using one of these ways.

Assessed type

Oracle

[c4-judge]

0xleastwood marked the issue as primary issue

[c4-judge]

0xleastwood marked the issue as selected for report

[0xStalin-eth]

Hello Judge @0xleastwood

I'd like to add a comment to this report to dispute the severity assigned to it.

First of all, important to clarify that the only Chainlink Oracle that would be used in the protocol could be (if its not used a Pyth Oracle instead, see this comment for the reason why ETH/ETH pairs are not used) the ETH/USD Oracle.

• This is not some derivative pair as LUNA, this is the backbone of the whole defi infrastructure. The odds of getting the price of such a pair to go outside the min and maxAnswer are equally or even lower than the odds of the multisig blocking access to the price itself. As such, I think the same criteria should also be applied to this report.

[ickas]

Fixed → https://github.com/baker-fi/bakerfi-contracts/pull/46

[t0x1cC0de]

Please refer my comments under #15 and also the fact that such issues have been marked as Medium in the past.

Thanks

[0xleastwood]

This is a little more severe than #15 because it doesn't simply mean that the protocol reverts, but it reports incorrect pricing which allows users to perform actions at an advantage. Leaving as is.