code-423n4 / 2024-05-predy-findings

10 stars 9 forks source link

Risk of Incorrect Price Feeds Due to Chainlink Oracle Circuit Breaker Activation #46

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/PriceFeed.sol#L45

Vulnerability details

Impact

The getSqrtPrice function in the PriceFeed contract is vulnerable to returning incorrect prices if the Chainlink oracle's circuit breaker mechanism is triggered. This can lead to users Trade the market against assets at incorrect prices, potentially causing significant financial losses and instability in the protocol. This scenario occurred on Venus on the Binance Smart Chain (BSC) during the collapse of LUNA.

Proof of Concept

When using the latestRoundData() the price of an asset deviates significantly from a predefined price range, Chainlink aggregators activate a circuit breaker mechanism. This mechanism causes the oracle to consistently return the minimum price instead of the actual price of the asset.

Consequently, users can continue to Trade the asset, but at an incorrect price.

For instance, consider TokenA with a minPrice set at $1. If the price of TokenA drops to $0.10, the aggregator still reports $1. This scenario enables users to Trade significant amounts of token, potentially leading to bankruptcy for the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

getSqrtPrice() should check the returned answer against the minPrice/maxPrice and revert if the answer is outside of the bounds:

(, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData()

++ if (quoteAnswer >= maxPrice or quoteAnswer <= minPrice) revert();

Assessed type

Oracle

alex-ppg commented 4 months ago

The submission and its duplicates detail how Chainlink price measurements do not have an upper or lower bound imposed on them, permitting any price reported by the Chainlink oracles to be consumed.

It is impossible to reliably configure a maximum and minimum price for a cryptocurrency asset due to its inherently volatile nature, and better alternatives would be to impose a maximum price deviation threshold based on time.

All submissions detail a fixed maximum and minimum value being imposed which is not a valid approach, and in general volatility security measures for off-chain oracles in particular may ultimately prove unhelpful, such as in the Luna price crash whereby Chainlink simply stopped reporting prices instead of reporting flash-crash measurements.

c4-judge commented 4 months ago

alex-ppg marked the issue as unsatisfactory: Invalid

0xAbhay commented 4 months ago

Hey @alex-ppg , Thank you for your feedback on the submitted vulnerability regarding the getSqrtPrice function in the PriceFeed contract.

You can read it here. this finding also follows the medium severity rule. In a recent C4 Noya contest, the same finding was given a medium severity rating here. Also the same issue given a medium severity issue in recent contest

for mitigation, we can use a combined Approach

Dynamic Deviation Thresholds: Implement a mechanism that checks if the current price deviates significantly from recent historical prices within a certain time window.

Predefined Bounds as Fallback: Use predefined minimum and maximum bounds as a secondary check to catch extreme outliers.

Off-Chain Monitoring: Implement off-chain monitoring to compare on-chain prices with other trusted data sources and disable the price feed if significant discrepancies are found.

alex-ppg commented 4 months ago

Hey @0xAbhay, thank you for sharing additional context in relation to this submission. While I appreciate what other judges have historically reviewed this submission as, I have detailed in my response why the recommended mitigation is (and was, historically) insufficient and would provide a false sense of security.

Given that it is impossible to evaluate the maximum and minimum price an oracle should enforce in the cryptocurrency market, I cannot consider this submission as a valid HM vulnerability.