Wrong Chainlink PriceFeeds implementation - Use of deprecated function and stale prices checks missing

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/oracles/ChainlinkPriceOracle.sol#L38

Vulnerability details

The protocol relies on Chainlink price feeds to retrieve the Asset/ETH price. However, the current implementation of Chainlink within the protocol is poor, leading to potential issues such as zero prices for assets. This is primarily attributed to the utilization of the deprecated latestAnswer() function to fetch prices. Additionally, the protocol lacks a mechanism to ensure the timeliness of price data are not stale.

    function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
        ///@audit-issue M poor chainlink implementation
        return AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
    }

Impact

• Zero Prices for the Assets

Proof-of-Concept

1. According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0.

2. The Dangers of Price Oracles

Tools Used

Shaheeniyat 🦅

Recommended Mitigation Steps

First use latestRoundData():

-    return AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
+    return (, int256 price, , , ) = priceFeed.latestRoundData();

Then add these 5 essential checks to make the system more robust:

1. It should make sure that price is greater than zero
2. startedAt value is greater than zero
3. The returned price is not older than 24 hours using startedAt
4. Round Completeness
5. try/catch to handle chainlink errors and use fallback oracle

function getAssetPrice(address asset) external view override returns (uint256) {
    try AggregatorInterface(assetPriceFeed[asset]).latestRoundData() returns (uint80 roundID, int256 price, uint256 startedAt, uint256 timeStamp, uint80 answeredInRound) {
        require(price > 0, "Invalid price");
        require(startedAt > 0, "Invalid startedAt");
        require(block.timestamp - startedAt < 1 days, "Price is outdated");
        require(answeredInRound >= roundID, "Round not complete");

        return uint256(price) * 1e10;
    } catch Error(string memory) {            
            // handle failure here:
            // revert, call proprietary fallback oracle, fetch from another 3rd-party oracle, etc.
        }
}

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #34

[c4-pre-sort]

raymondfam marked the issue as not a duplicate

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #194

[c4-pre-sort]

raymondfam marked the issue as duplicate of #723

[c4-judge]

fatherGoose1 changed the severity to QA (Quality Assurance)

[c4-judge]

fatherGoose1 marked the issue as grade-b