Skip to content

Check for previous Chainlink price using `roundId`

Low
RickGriff published GHSA-jcf4-j6h6-xw2r Dec 8, 2023

Package

No package listed

Affected versions

v1.0

Patched versions

None

Description

As part of the price sanity check in PriceFeed.sol, Liquity fetches the latest price from Chainlink and the price before the latest, in order to compare the magnitude of the deviation between consecutive Chainlink price updates.

The Chainlink ETH-USD feed operates via a “round” system - nodes periodically submit price data and the data is aggregated off-chain by Chainlink’s oracle network. A new round occurs when the aggregate price is pushed on-chain to the aggregator contract, to be consumed by dApps like Liquity.

Liquity identifies Chainlink price updates by roundId. Liquitry defines the current Chainlink price as the one with the latest roundId, and the previous Chainlink price as the price with a round ID of roundId - 1. Here is the Liquity code where the previous Chainlink price is fetched in the ​​_getPrevChainlinkResponse function in PriceFeed.sol:

function _getPrevChainlinkResponse(uint80 _currentRoundId, uint8 _currentDecimals) internal view returns (ChainlinkResponse memory prevChainlinkResponse) {
/*
* NOTE: Chainlink only offers a current decimals() value - there is no way to obtain the decimal precision used in a
* previous round. We assume the decimals used in the previous round are the same as the current round.
*/
// Try to get the price data from the previous round:
try priceAggregator.getRoundData(_currentRoundId - 1) returns
(
uint80 roundId,
int256 answer,
uint256 /* startedAt */,
uint256 timestamp,
uint80 /* answeredInRound */
)
{
// If call to Chainlink succeeds, return the response and success = true
prevChainlinkResponse.roundId = roundId;
prevChainlinkResponse.answer = answer;
prevChainlinkResponse.timestamp = timestamp;
prevChainlinkResponse.decimals = _currentDecimals;
prevChainlinkResponse.success = true;
return prevChainlinkResponse;
} catch {
// If call to Chainlink aggregator reverts, return a zero response with success = false
return prevChainlinkResponse;
}
}

However, using roundId - 1 for the previous price assumes that the roundId always and only increases by 1 every time a new round occurs.

Impact

If this assumption did not hold and roundId could jump by >1 between price updates, then if Liquity tried to fetch the price with roundId - 1 when no Chainlink round with this value existed, this call to the Chainlink aggregator here would revert on L550 in the above function:

try priceAggregator.getRoundData(_currentRoundId - 1) returns

This revert would be caught by the try-catch statement, and Liquity will in turn decide that Chainlink has failed in the _chainlinkIsBroken function:

function _chainlinkIsBroken(ChainlinkResponse memory _currentResponse, ChainlinkResponse memory _prevResponse) internal view returns (bool) {
return _badChainlinkResponse(_currentResponse) || _badChainlinkResponse(_prevResponse);
}
function _badChainlinkResponse(ChainlinkResponse memory _response) internal view returns (bool) {
// Check for response call reverted
if (!_response.success) {return true;}
// Check for an invalid roundId that is 0
if (_response.roundId == 0) {return true;}
// Check for an invalid timeStamp that is 0, or in the future
if (_response.timestamp == 0 || _response.timestamp > block.timestamp) {return true;}
// Check for non-positive price
if (_response.answer <= 0) {return true;}
return false;
}

It would then fall back to using Tellor as the ETH-USD price oracle.

This scenario is undesirable - a delta in price round IDs greater than one on the (otherwise functional) Chainlink feed should not in itself trigger a switch to the fallback oracle.

Can roundId ever jump by >1 between two updates?

In normal times, the roundId increases by 1 at each new round.

According to Chainlink, the exception is when the aggregator logic contract is upgraded. Chainlink price feeds have a proxy architecture, and the proxy calculates the roundId based in part on a unique phaseId of the underlying aggregator.

When the aggregator is upgraded the roundId for consecutive price updates can differ by >1 due to the way the roundId is calculated based on the phaseId of the aggregator contract, which changes at every upgrade:
https://docs.chain.link/data-feeds/historical-data

This is evidenced here:
https://dune.com/queries/266838/3020619

However, in practice Chainlink sensibly do not immediately point the proxy to the new aggregator implementation. After deployment, the aggregator accrues a few hundred rounds of price data for quality control purposes before the proxy is pointed to the new aggregator contract.

This means that when Liquity fetches price data for the first time after an aggregator upgrade, both roundId and roundId - 1 will be valid round IDs and Liquity will successfully fetch both the the current and previous round price data (assuming Chainlink is otherwise functioning properly).

Severity

Low

CVSS overall score

This score calculates overall vulnerability severity from 0 to 10 and is based on the Common Vulnerability Scoring System (CVSS).
/ 10

CVSS v3 base metrics

Attack vector
Network
Attack complexity
High
Privileges required
None
User interaction
Required
Scope
Unchanged
Confidentiality
None
Integrity
Low
Availability
None

CVSS v3 base metrics

Attack vector: More severe the more the remote (logically and physically) an attacker can be in order to exploit the vulnerability.
Attack complexity: More severe for the least complex attacks.
Privileges required: More severe if no privileges are required.
User interaction: More severe when no user interaction is required.
Scope: More severe when a scope change occurs, e.g. one vulnerable component impacts resources in components beyond its security scope.
Confidentiality: More severe when loss of data confidentiality is highest, measuring the level of data access available to an unauthorized user.
Integrity: More severe when loss of data integrity is the highest, measuring the consequence of data modification possible by an unauthorized user.
Availability: More severe when the loss of impacted component availability is highest.
CVSS:3.0/AV:N/AC:H/PR:N/UI:R/S:U/C:N/I:L/A:N

CVE ID

No known CVE

Weaknesses

No CWEs