QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: low
Invalid

Conflicting primary oracle and backup oracle querying desired end result - to use stale data or not?

jsondoge

Summary

When _getData is called the oracles are queried for latest results. If primary oracles are stale, backups are used. And in the cyfrin report 7.2.4 the sponsor responded:

A stale update is better than none because of the way the estimators encapsulate a weighted price and a certain level of smoothing...

Yet the _getData will return stale data when there are no backup oracles and revert if backup oracles provided and all of them are stale.

Vulnerability Details

Currently the getData oracle fetching logic in summary is:

  • If only primary oracle exists -> it is up to date -> return UP TO DATE primary oracle data

  • If only primary oracle exists -> it is stale -> return STALE primary oracle data

  • If primary and backup oracles exist -> primary stale, but backup up to date -> return UP TO DATE backup oracle data

  • If primary and backup oracles exist -> primary stale and backup stale -> REVERT

The revert condition can be seen in _getData backup oracle loop:

for (uint j = 1 /*0 already done via optimised poolOracles*/; j < numAssetOracles; ) {
​
oracleResult = _getOracleData(
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"); // <- if final index reached and all backup oracles are stale
}
unchecked {
++j;
}
}
​

Since the sponsor stated that stale data update is better than none, the _getData should always return data, even if it is stale. Thus the final point creates a conflict and should never happen. In case both are stale quantAmm should decide if it wants to use primary oracle or the oracle with the latest data even if it stale.

Furthermore this is happening in a loop:

for (uint i; i < oracleLength; ) {
...
}

Which means even if some oracle returned stale data (no backups), another oracle with backups can still cause the whole function call to revert.

Impact

Conflicting existing logic will not return data (revert) if any backup oracles provided and they are all stale, but will return stale data if no backup oracles added. _getData is used in the main performUpdate, uplift hook deposit/withdraw/transfer flows. Same severity as the original Cyfrin report -> Medium.

Tools Used

Manual review

Recommendations

If data is prefered over reverting, then _getData should never revert. And in case all backup oracles are stale: use primary oracle or the oracle with the latest data even if it stale.

Another example to simply return the last fetched oracle data:

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");
+ break;
}

Extra Notes

Keep in mind that when stale data will always be returned it will need a new check against the following:

function _getOracleData(OracleWrapper _oracle) private view returns (OracleData memory oracleResult) {
if (!approvedOracles[address(_oracle)]) return oracleResult; // @audit This data will need to be extra checked outside
(int216 data, uint40 timestamp) = _oracle.getData();
oracleResult.data = data;
oracleResult.timestamp = timestamp;
}

So you'll need a check like this before choosing the final output

oracleResult.data != 0 && oracleResult.timestamp != 0

Since in a case where both main and backup return stale data one of them could be simply disapproved.

Updates

Lead Judging Commences

n0kto Lead Judge 7 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

jsondoge Submitter
7 months ago
n0kto Lead Judge
7 months ago
n0kto Lead Judge 7 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

Support

FAQs

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