Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Getting prices would revert in some valid instances

bauchibred

Summary

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/external/chainlink/ChainlinkUtil.sol#L26-L77

function getPrice(
IAggregatorV3 priceFeed,
uint32 priceFeedHeartbeatSeconds,
IAggregatorV3 sequencerUptimeFeed
)
internal
view
returns (UD60x18 price)
{
..snip
try priceFeed.latestRoundData() returns (uint80, int256 answer, uint256, uint256 updatedAt, uint80) {
if (block.timestamp - updatedAt > priceFeedHeartbeatSeconds) {
revert Errors.OraclePriceFeedHeartbeat(address(priceFeed));
}
​
IOffchainAggregator aggregator = IOffchainAggregator(priceFeed.aggregator());
int192 minAnswer = aggregator.minAnswer();
int192 maxAnswer = aggregator.maxAnswer();
​
if (answer <= minAnswer || answer >= maxAnswer) {
revert Errors.OraclePriceFeedOutOfRange(address(priceFeed));
}
​
price = ud60x18(answer.toUint256() * 10 ** (Constants.SYSTEM_DECIMALS - priceDecimals));
} catch {
revert Errors.InvalidOracleReturn();
}
}
​

This function is used to query the provided Chainlink Price Feed for the margin collateral oracle price, and it's been done in a try/catch format in order to better bubble up the errors/failures, issue however is that not all error cases are being checked against.

This is because protocol currently assumes that the minAnswer(), maxAnswer() functionality is always going to lead to valid returns.

However not all feeds support this, which would then mean that even if the feed is a valid one and the call to priceFeed.latestRoundData() does not fail, when checking for the min/maxAnswer this query could fail if not supported, however this price should be ingested in as much as it is not stale, but due to current implementation the whole attempt reverts.

Impact

Likelihood is medium, considering not all feeds support the min/maxAnswer, however impact is high cause this functionality is queried when tryig to get the margin collateral oracle price which is needed in core areas of the protocol.

Tools Used

Manual review

Recommendations

Consider also wrapping the min/maxAnswer queries in a try/catch a

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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