QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: medium
Valid

Reserved oracles can be useless in case of revert in the `getData()`

pontifex

Summary

The protocol utilizes reserved oracles for additional fault tolerance. But in case of reverting in the OracleWrapper.getData function the other reserved oracles are not called.

Vulnerability Details

The UpdateWeightRunner._getData can throw an error when "No fresh oracle values available". As mentioned in the comment this should rarely happen. But in fact reverts OracleWrapper.getData function can interrupt the function execution early:

function _getData(address _pool, bool internalCall) private view returns (int256[] memory outputData) {
require(internalCall || (approvedPoolActions[_pool] & MASK_POOL_GET_DATA > 0), "Not allowed to get data");
//optimised == happy path, optimised into a different array to save gas
address[] memory optimisedOracles = poolOracles[_pool];
uint oracleLength = optimisedOracles.length;
uint numAssetOracles;
outputData = new int256[](oracleLength);
uint oracleStalenessThreshold = IQuantAMMWeightedPool(_pool).getOracleStalenessThreshold();
​
for (uint i; i < oracleLength; ) {
// Asset is base asset
OracleData memory oracleResult;
>> oracleResult = _getOracleData(OracleWrapper(optimisedOracles[i]));
if (oracleResult.timestamp > block.timestamp - oracleStalenessThreshold) {
outputData[i] = oracleResult.data;
} else {
unchecked {
numAssetOracles = poolBackupOracles[_pool][i].length;
}
​
for (uint j = 1 /*0 already done via optimised poolOracles*/; j < numAssetOracles; ) {
>> oracleResult = _getOracleData(
// poolBackupOracles[_pool][asset][oracle]
OracleWrapper(poolBackupOracles[_pool][i][j])
);
if (oracleResult.timestamp > block.timestamp - oracleStalenessThreshold) {
// Oracle has fresh values
break;
} else if (j == numAssetOracles - 1) {
// All oracle results for this data point are stale. Should rarely happen in practice with proper backup oracles.
​
>> revert("No fresh oracle values available");
}
unchecked {
++j;
}
}
outputData[i] = oracleResult.data;
}
​
unchecked {
++i;
}
}
}

OracleWrapper.sol

function getData() public view returns (int216 data, uint40 timestamp) {
(data, timestamp) = _getData();
>> require(timestamp > 0, "INVORCLVAL"); // Sanity check in case oracle returns invalid values
}

ChainlinkOracle.sol

function _getData() internal view override returns (int216, uint40) {
(, /*uint80 roundID*/ int data, , /*uint startedAt*/ uint timestamp, ) = /*uint80 answeredInRound*/
priceFeed.latestRoundData();
>> require(data > 0, "INVLDDATA");
data = data * int(10 ** normalizationFactor);
return (int216(data), uint40(timestamp)); // Overflow of data is extremely improbable and uint40 is large enough for timestamps for a very long time
}

Impact

Temporary blocking core functionality, unexpected behavior.

Tools used

Manual Review

Recommendations

Consider return zero value in the oracleResult variable instead of revert.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

invalid_stale_price_when_no_backup_oracles_set

Cyfrin audit: 7.2.4 Stale Oracle prices accepted when no backup oracles available

Appeal created

pontifex Submitter
10 months ago
0xwsecteam Auditor
10 months ago
pontifex Submitter
10 months ago
n0kto Lead Judge
9 months ago
n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_no_try_catch_for_Chainlink_oracle

Likelihood: Low, price feed should revert. Impact: High, DoS of the protocol

Support

FAQs

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