0xb0k0 - `latestRoundData()` checks in the ChainLink oracles does not check for round completeness

[sherlock-admin2]

0xb0k0

Medium

latestRoundData() checks in the ChainLink oracles does not check for round completeness

Summary

Using ChainLink oracles without accounting for round completeness can lead to using stale prices. This in turn will cause all functions that rely on these prices to operate outside of the normal behaviour.

Vulnerability Detail

Sentiment utilizes the ChainLink price oracles to obtain information regarding ETH/ERC20 prices. The latestRoundData() method is invoked to obtain this data. Currently, only the returned price and timestamps are checked, where the round completeness check is missing.

According to Chainlink's documentation, this function does not throw an error if no answer can be obtained but instead returns 0, or in the worst case stale data. This means that if the underlying system using these feeds does not account for all cases, there could be incorrect behavior. For example, the oracle could fall behind or otherwise fail to be maintained, resulting in outdated data being fed to the index price calculations. Oracle reliance has historically resulted in crippled on-chain systems. Such complications that lead to these outcomes can arise from things as simple as network congestion.

Impact

If there is a problem with chainlink starting a new round and/or finding consensus on the new value for the oracle (e.g. chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the chainlink system) consumers of this contract may continue using outdated stale data (if oracles are unable to submit no new round is started).

This could lead to stale prices, wrong price return values, and as a whole outdated data.

As a result, the functions that rely on accurate price feed might not work as expected, which sometimes leads to fund loss. The impacts vary and depend on the specific situation like the following:

• incorrect liquidation
  • some users could be liquidated when they should not
  • no liquidation is performed when there should be
• wrong price feed
  • causing inappropriate loans to be taken, beyond the current collateral factor
  • too low price feed affects normal bor

Code Snippet

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/25a0c8aeaddec273c5318540059165696591ecfb/protocol-v2/src/oracle/ChainlinkEthOracle.sol#L101 https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/25a0c8aeaddec273c5318540059165696591ecfb/protocol-v2/src/oracle/ChainlinkUsdOracle.sol#L116

Tool used

Manual Review

Recommendation

Enforce round completeness checks:

diff --git a/protocol-v2/src/oracle/ChainlinkEthOracle.sol b/protocol-v2/src/oracle/ChainlinkEthOracle.sol
index 27dda5d..f340a3b 100644
--- a/protocol-v2/src/oracle/ChainlinkEthOracle.sol
+++ b/protocol-v2/src/oracle/ChainlinkEthOracle.sol
@@ -98,9 +98,10 @@ contract ChainlinkEthOracle is Ownable, IOracle {
     /// @dev Fetch price from chainlink feed with sanity checks
     function _getPriceWithSanityChecks(address asset) private view returns (uint256) {
         address feed = priceFeedFor[asset];
-        (, int256 price,, uint256 updatedAt,) = IAggegregatorV3(feed).latestRoundData();
+        (uint80 roundId, int256 price,, uint256 updatedAt, uint80 answeredInRound) = IAggegregatorV3(feed).latestRoundData();
         if (price <= 0) revert ChainlinkEthOracle_NonPositivePrice(asset);
         if (updatedAt < block.timestamp - stalePriceThresholdFor[feed]) revert ChainlinkEthOracle_StalePrice(asset);
+        if (answeredInRound < roundId) revert ChainlinkEthOracle_StalePrice(asset);
         return uint256(price);
     }
 }

 diff --git a/protocol-v2/src/oracle/ChainlinkUsdOracle.sol b/protocol-v2/src/oracle/ChainlinkUsdOracle.sol
index ecfb06e..039ff6e 100644
--- a/protocol-v2/src/oracle/ChainlinkUsdOracle.sol
+++ b/protocol-v2/src/oracle/ChainlinkUsdOracle.sol
@@ -111,11 +111,13 @@ contract ChainlinkUsdOracle is Ownable, IOracle {
     }

     /// @dev Fetch price from chainlink feed with sanity checks
     function _getPriceWithSanityChecks(address asset) private view returns (uint256) {
         address feed = priceFeedFor[asset];
-        (, int256 price,, uint256 updatedAt,) = IAggegregatorV3(feed).latestRoundData();
+        (uint80 roundId, int256 price,, uint256 updatedAt, uint80 answeredInRound) = IAggegregatorV3(feed).latestRoundData();
         if (price <= 0) revert ChainlinkUsdOracle_NonPositivePrice(asset);
         if (updatedAt < block.timestamp - stalePriceThresholdFor[feed]) revert ChainlinkUsdOracle_StalePrice(asset);
+        if (answeredInRound < roundId) revert ChainlinkUsdOracle_StalePrice(asset);
         return uint256(price);
     }
 }

[sherlock-admin4]

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

z3s commented:

Invalid; answeredInRound is Deprecated