The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Insufficient validation of values returned by Chainlink `AggregatorV3Interface::latestRoundData` and unhandled reverts could block all liquidations

Ciara and Gio

Description

LiquidationPool::distributeAssets uses AggregatorV3Interface::latestRoundData to return Chainlink price data on lines 207 and 218. These calls can potentially revert which will cause the liquidation transaction itself to revert and result in a state of temporary DoS (since feed addresses can be updated on the TokenManager contract if needed).

(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
...
(,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();

Additionally, these calls do not check whether the return values indicate stale data or round completeness. This could lead to the reporting of incorrect prices according to the Chainlink documentation, which states that this function does not error if no answer has been reached but instead returns 0 or outdated round data. If the call does not revert, but there is an issue with the current round, it is possible that latestRoundData returns an invalid price, e.g. 0.

If it is not possible to fetch a valid price for assetPriceUsd and this returns zero, then stakers will be rewarded a share of the liquidated assets at no cost in Euros. If it is not possible to fetch a valid price for priceEurUsd and this returns zero, then the transaction will revert due to division by zero, leading to the same outcome as above. Therefore, it is important to sufficiently validate the other values returned by latestRoundData to ensure price data is not stale or otherwise invalid.

uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd) * _hundredPC / _collateralRate;

This issue is also present in the price calculator (out of scope) and should be mitigated accordingly.

Impact

This issue has the potential to cause bad debt to accrue within the protocol due to the temporary blocking of liquidations and an error in protocol accounting. The impact is high, and the likelihood is low – but note: very possible – so this is a medium-severity finding.

Recommended Mitigation

  • Always query Chainlink price feeds within a try/catch block. If the call to the price feed fails, execution of the transaction will continue, and the revert can be handled:

try AggregatorV3Interface(priceFeedAddress).latestRoundData() returns (
uint80 roundID,
int256 price,
uint256 startedAt,
uint256 timestamp,
uint80 answeredInRound
) {
return (roundID, price, startedAt, timestamp, answeredInRound);
} catch Error(string memory) {
// handle failure here:
// revert, call propietary fallback oracle, fetch from another 3rd-party oracle, etc.
}

  • Always validate that data feed for staleness and round completeness:

(
uint80 roundID,
int signedPrice,
/*uint startedAt*/,
uint timeStamp,
uint80 answeredInRound
) = priceFeed.latestRoundData();
require(signedPrice > 0, "Negative Oracle Price");
require(timeStamp >= block.timestamp - HEARTBEAT_TIME , "Stale price feed");
require(signedPrice < _maxPrice, "Upper price bound breached");
require(signedPrice > _minPrice, "Lower price bound breached");
require(answeredInRound >= roundID, "Round incomplete");

Combined, this might look like:

try AggregatorV3Interface(priceFeedAddress).latestRoundData() returns (
uint80 roundID,
int256 price,
uint256 startedAt,
uint256 timestamp,
uint80 answeredInRound
) {
require(signedPrice > 0, "Negative Oracle Price");
require(timeStamp >= block.timestamp - HEARTBEAT_TIME , "Stale price feed");
require(signedPrice < _maxPrice, "Upper price bound breached");
require(signedPrice > _minPrice, "Lower price bound breached");
require(answeredInRound >= roundID, "Round incomplete");
return price;
} catch Error(string memory) {
// handle failure here:
// revert, call propietary fallback oracle, fetch from another 3rd-party oracle, etc.
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Chainlink-price

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Chainlink-price

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.