search

code-423n4 / 2024-05-bakerfi-findings

4 stars 4 forks source link

min and maxAnswer never checked for oracle price feed #16

Open c4-bot-10 opened 3 months ago

c4-bot-10 commented 3 months ago

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

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 commented 3 months ago

0xleastwood marked the issue as primary issue

c4-judge commented 3 months ago

0xleastwood marked the issue as selected for report

stalinMacias commented 3 months ago

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.

ickas commented 3 months ago

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

t0x1cC0de commented 3 months ago

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

Thanks

0xleastwood commented 3 months ago

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.