devScrooge - No check if Chainlink's function .latestsRoundData returns stale prices.

[sherlock-admin]

devScrooge

medium

No check if Chainlink's function .latestsRoundData returns stale prices.

Summary

When a Chainlink aggregtor is used for getting a price using latestRoundData the return values are not check so they can be stale prices.

Vulnerability Detail

The PriceOracles.sol contract implement several functions does call to the latestsRoundData provided by Chainlink. The functions where latestsRoundData is called are getPriceFromChainlink and _setAggregators. Once the values are returned, the only check done is require(price > 0, "invalid price");. However this check does not guarantee that the returned price is not an stale price.

I am adding some other sources where this vulnerability is described:

1. https://solodit.xyz/issues/9795
2. Should check return data from Chainlink aggregators code-423n4/2021-05-fairside-findings#70
3. https://code4rena.com/reports/2022-12-tigris#m-24-chainlink-price-feed-is-not-sufficiently-validated-and-can-return-stale-price

Impact

The returned prices can be stale prices, this means that the prices uses for the protocol may not be the correct ones. These values that are returned must pass additional checks before proceeding with price updates. If you only receive the price without considering these values, there is a possibility of encountering outdated prices and incomplete data rounds.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/oracle/PriceOracle.sol#L107
https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/oracle/PriceOracle.sol#L67

Tool used

Manual Review

Recommendation

1. Get the rest of returned values:

(uint80 roundId, int256 price, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) = registry.latestRoundData(base, quote);

2. Add some checks for the returned values:

require ( price > 0);
require ( answeredInRound >= roundID, "Stale price");
require ( updatedAt != 0, "Round not complete");

3. It is also recommended to set a 'threshold' variable to check the updatedAt value:

threshold <= block.timestamp - updatedAt

Duplicate of #9