Lack of validity and sanity checks while consuming Chainlink Oracle data

Summary

The project does not follow best practises in consuming Chainlink Oracle Price Feeds Data The result from Chainlink latestRoundData() are not validated or checked for integrity

Vulnerability Detail

The price received from latestRoundData() for the price of ETH in terms of the keeper token will be immediately used without any checks to the various other critical data returned from call. All other data for integrity checks are ignored.

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

See 0xMacro how to consume Oracle Price Feeds Safely -> https://0xmacro.com/blog/how-to-consume-chainlink-price-feeds-safely/

Impact

(uint80 roundID, int256 answer, , uint256 timestamp, uint80 answeredInRound) = ethTokenOracleFeed().latestRoundData(); This implies there are no checks for the following

if the price is zero
if the data is stale
if the price may be incorrect or reasonable
if the round was complete Faulty prices, no prices, stale prices or zero prices impact working of protocol especially the critical incentives for the Keepers which are integral to the protocol

Code Snippet

/// @notice Returns the price of ETH in terms of the keeper token
    /// @return The price of ETH in terms of the keeper token
    function _etherPrice() private view returns (UFixed18) {
        (, int256 answer, , ,) = ethTokenOracleFeed().latestRoundData();
        return UFixed18Lib.from(Fixed18Lib.ratio(answer, 1e8)); // chainlink eth-usd feed uses 8 decimals
    }

Tool used

Manual Review

Recommendation

It is recommended to perform checks for the returned data and ensure data validity and integrity by following the best practises of consuming Chianlinke Oracle Data here -> See 0xMacro how to consume Oracle Price Feeds Safely -> https://0xmacro.com/blog/how-to-consume-chainlink-price-feeds-safely/
Checks recommended in link focus on price validity, roundId validity, timestamp validity, price not zero, price in reasonable deviation, storing last good values and staleness checks, circuit breaker suggestions etc

For example below checks can be made as a bare minimum ...

   (uint80 roundID, int256 answer, , uint256 timestamp, uint80 answeredInRound) = ethTokenOracleFeed()latestRoundData();
        require(roundID !=  0, "Invalid");
        require(answeredInRound >= roundID, "Stale price");
        require(timestamp != 0,"Round not complete");
        require(answer > 0,"Chainlink answer reporting 0");
        ........

sherlock-audit / 2023-07-perennial-judging

MatricksDeCoder - Lack of validity and sanity checks while consuming Chainlink Oracle data #120