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

Getting data from pool can be reverted when one of the oracle is not live

zukanopro

Vulnerability Details

In _getData() function in UpdateWeightRunner contract, it use _getOracleData() function to get data from oracle:

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;
}
}
}

It will call to getData() function of oracle:

function _getOracleData(OracleWrapper _oracle) private view returns (OracleData memory oracleResult) {
if (!approvedOracles[address(_oracle)]) return oracleResult; // Return empty timestamp if oracle is no longer approved, result will be discarded
(int216 data, uint40 timestamp) = _oracle.getData(); // <---
oracleResult.data = data;
oracleResult.timestamp = timestamp;
}

getData() function in OracleWrapper abstract contract:

function getData() public view returns (int216 data, uint40 timestamp) {
(data, timestamp) = _getData();
require(timestamp > 0, "INVORCLVAL"); // <--
}

_getData() function in ChainlinkOracle contract:

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

It can be seen that there are 2 scenario that will lead to revert in getData() and _getData() function, one of them can be happened in the real scenario, which is noted by chainlink's dev at here link

*
* @dev A timestamp with zero value means the round is not complete and should not be used.
*/

So, when one of the oracle requested revert, function getData() in UpdateWeightRunner contract will revert, lead to DoS in multiple functions that need to get data from oracle. It is a big problem for smoothing of protocol, Which mentioned in issue M-4 of the report link: https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-quantamm/audit/Cyfrin_1_2_20241217.pdf

Recommendations

Move these checking conditions to _getData() function in UpdateWeightRunner contract when calling oracle, or using try - catch when call _getOracleData() function

Updates

Lead Judging Commences

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