As stale price is determined by time since last timestamp, the price that is most recent, but wasn't updated for more than 2 hours (say there were no trades on the market) will be rejected, which makes system functionality unavailable in such a case. This can be deemed too conservative, but that's an architectural choice.
However, in the same time real stale price will be accepted as long as it happens to be in the last 2 hours window. I.e. ChainlinkOracleProvider do not require the price to be the most recent, simply accepting any price given that it is not older than 2 hours.
Another issue is that zero price, which in the most cases will indicate Oracle malfunction, will be accepted by the system. I.e. devalued tokens usually do have positive, albeit very small, price.
chase-manning
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L58
Vulnerability details
Impact
As stale price is determined by time since last timestamp, the price that is most recent, but wasn't updated for more than 2 hours (say there were no trades on the market) will be rejected, which makes system functionality unavailable in such a case. This can be deemed too conservative, but that's an architectural choice.
However, in the same time real stale price will be accepted as long as it happens to be in the last 2 hours window. I.e. ChainlinkOracleProvider do not require the price to be the most recent, simply accepting any price given that it is not older than 2 hours.
Another issue is that zero price, which in the most cases will indicate Oracle malfunction, will be accepted by the system. I.e. devalued tokens usually do have positive, albeit very small, price.
Proof of Concept
getPriceUSD allows for zero price:
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L58
Oracle prices are used as a denominator across the system, for example:
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L47
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/StrategySwapper.sol#L319-L319
Recommended Mitigation Steps
Consider imposing checks for positive price and for the round id, timestamp fields:
https://docs.chain.link/docs/feed-registry/#latestrounddata