search

sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

MohammedRizwan - Chainlink oracle will return the wrong price if the aggregator hits minAnswer #151

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

MohammedRizwan

medium

Chainlink oracle will return the wrong price if the aggregator hits minAnswer

Summary

Chainlink oracle will return the wrong price if the aggregator hits minAnswer

Vulnerability Detail

Impact

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


    function _etherPrice() private view returns (UFixed18) {
        (, int256 answer, , ,) = ethTokenOracleFeed().latestRoundData();
        return UFixed18Lib.from(Fixed18Lib.ratio(answer, 1e8)); // chainlink eth-usd feed uses 8 decimals
    }

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/root/contracts/attribute/Kept.sol#L62

Tool used

Manual Review

Recommendation

Consider using the following checks.

For example:

(uint80, int256 answer, uint, uint, uint80) = oracle.latestRoundData();

// minPrice check
require(answer > minPrice, "Min price exceeded");
// maxPrice check
require(answer < maxPrice, "Max price exceeded");
sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

141345 commented:

m

n33k commented:

unhandled stale price returned from latestRoundData()

arjun-io commented 1 year ago

Similar to https://github.com/sherlock-audit/2023-07-perennial-judging/issues/159 we should not block keepers from receiving incentives, and instead we should simply use the best estimate of a price that Chainlink provides. Also, there is no accounting or collateral pricing based on these prices