c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The issue is highlighted in the bot L-2 finding but fail to highlight the importance for checking stale price. The ChainlinkPriceOracle when calls out to a Chainlink oracle receiving using the recommended latestRoundData() it can get stale price, if there is a problem with Chainlink starting a new round and finding consensus on the new value for the oracle (e.g. Chainlink nodes abandon the oracle, high volatility ) consumers of this contract may continue using outdated stale or incorrect data (if oracles are unable to submit no new round is started).

Proof of Concept

The recommendation of using latestRoundData in the bot finding does not provide consideration for checking updated price feed which can result in loss of funds for user and protocol.

function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
        return AggregatorInterface(assetPriceFeed[asset]).latestRoundData();
    }

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to check for price feed with an interval of 1 hour or more as desired by the protocol to have updated price for every asset.

function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
-        return AggregatorInterface(assetPriceFeed[asset]).latestAnswer();

+   (uint80 roundID, int256 answer, uint256 timestamp, uint256 updatedAt, ) = return AggregatorInterface(assetPriceFeed[asset]).latestRoundData
 ();

+       require(updatedAt >= roundID, "Stale price");
+      require(timestamp != 0,"Round not complete");
+        require(answer > 0,"Chainlink answer reporting 0");

+      if (updatedAt < block.timestamp – 1 hours)
+         revert PRICE_OUTDATED(assetPriceFeed[asset]);

}