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

Redundant price range validation in `ChainlinkUtil::getPrice` function

0xe4669da

Summary

In ChainlinkUtil::getPrice function, the price range check implementation is useless because it is already done by OffchainAggregator.sol in Chainlink codebase.

Vulnerability Details

In LightChaserV3 report: Medium-5, this known issue does not highlight the actual problems in ChainlinkUtil::getPrice function. Merely just saying that price is not validated is a vague finding.

The issue that I am reporting here pertains to the implementation of and reliance on a redundant price range check in ChainlinkUtil::getPrice function. Because this check is already implemented by Chainlink itself in OffchainAggregator::transmit function.

Furthermore, the issue here is that we are fetching a value from a source and then validating the fetched value against other values fetched from the same source.

Source:ChainlinkUtil.sol#L59C1-L75C10

...
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();
}
...

Impact

The computation and logical implementation of price of collateral assets sits at the heart of a perpetuals DEX protocol like Zaros.

As a consequence of wrong collateral asset price computations, positions may be liquidated prematurely or fail to liquidate when they should and eventually resulting bad debts. Positions open on wrong collateral prices which may cost major financial losses to the protocol. Incorrect collateral valuation can cause undercollateralization.

PoC

Below is the code from Chainlink OffchainAggregator::transmit function. OffchainAggregator.sol#L681 has implemented this check beforehand.

contract OffchainAggregator is Owned, OffchainAggregatorBilling, AggregatorV2V3Interface, TypeAndVersionInterface {
​
function transmit(
// NOTE: If these parameters are changed, expectedMsgDataLength and/or
// TRANSMIT_MSGDATA_CONSTANT_LENGTH_COMPONENT need to be changed accordingly
bytes calldata _report,
bytes32[] calldata _rs, bytes32[] calldata _ss, bytes32 _rawVs // signatures
)
external
{
....
​
@> require(minAnswer <= median && median <= maxAnswer, "median is out of min-max range");
​
....
​
}
​
}
​

Tools Used

Manual review

Recommendations

Existing price range check should be removed and range should be validated either by using the values retreived from another external source or values set in the configurations by the owner.

...
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));
-- }
​
++ if (answer < minPriceConfigValue || answer > maxPriceConfigValue) {
++ revert Errors.OraclePriceFeedOutOfRange(address(priceFeed));
++ }
​
price = ud60x18(answer.toUint256() * 10 ** (Constants.SYSTEM_DECIMALS - priceDecimals));
} catch {
revert Errors.InvalidOracleReturn();
}
...
​
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

0xe4669da Submitter
12 months ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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