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

Removal of an Oracle in a 2 - Oracle Pool will cause problems

angrymustacheman

Summary

UpdateWeightRunner contract has a functionality to disapprove an Oracle from being used . It would cause issues , if removed oracle was the only backup oracle of the Pool and the optimized oracle is giving a stale value . In such situations important operations like performing updates in UpdateWeightRunner & operations like adding liquidity , removing liquidity & even transferring of lpNft in UpliftOnlyExample.sol get's paused temporarily till the Optimized Oracle returns an updated value .

Vulnerability Details

UpdateWeightRunner contract has this removeOracle function where approvedOracles mapping can be set to false :
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L217-L223

/// @notice Removes an existing oracle from the approved oracles
/// @param _oracleToRemove The oracle to remove
function removeOracle(OracleWrapper _oracleToRemove) external {
approvedOracles[address(_oracleToRemove)] = false;
require(msg.sender == quantammAdmin, "ONLYADMIN");
emit OracleRemved(address(_oracleToRemove));
}

The private scoped _getOracleData uses it before returning any data from oracle & skips the execution by returning empty struct if it's turned off as can be seen here :

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L329-L336

/// @notice Call oracle to retrieve new data
/// @param _oracle the target 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/* <@audit */
(int216 data, uint40 timestamp) = _oracle.getData();
oracleResult.data = data;
oracleResult.timestamp = timestamp;
}

This is the function that is used in _getData function to get the data of different tokens of a pool as can be seen here :
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L338-L383

/// @notice Wrapper for if someone wants to get the oracle data the rule is using from an external source
/// @param _pool Pool to get data for
function getData(address _pool) public view virtual returns (int256[] memory outputData) {
return _getData(_pool, false);
}
​
/// @notice Get the data for a pool from the oracles and return it in the same order as the assets in the pool
/// @param _pool Pool to get data for
/// @param internalCall Internal call flag to detect if the function was called internally for emission and permissions
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) {/* < @audit - if 1st oracle stale, the goes to backup oracle */
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( /* < @audit - empty struct returned as backup oracle was removed by admin */
// 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");/* < @audit - here it reverts */
}
unchecked {
++j;
}
}

The problem is that the public getData function and private _getData function are used in a lot of important operations and reverts in the function causes temporary outage for such operations :

here

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L242-L248

//this requires the pool to be registered with the QuantAMM update weight runner
//as well as approved with oracles that provide the prices
uint256 depositValue = getPoolLPTokenValue(
IUpdateWeightRunner(_updateWeightRunner).getData(pool), /* <@audit */
pool,
MULDIRECTION.MULDOWN
);

& here :

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L452

IUpdateWeightRunner(_updateWeightRunner).getData(pool),

& here :
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L585-L587

}
​
int256[] memory prices = IUpdateWeightRunner(_updateWeightRunner).getData(poolAddress);

& here :

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L396-L398

PoolRuleSettings memory _ruleSettings
) private returns (int256[] memory updatedWeights, int256[] memory data) {
data = _getData(_pool, true);

  • The main reason why this causes an issue is because there is no way add another approved Oracle to the poolBackupOracles mapping for Pool Owner or UpdateWeightRunner owner to resolve this issue . This issue can also happen in the case where an optimisedOracles of the pool has been removed and the only backup Oracle is returning stale values .

Protocol is supposed to deployed on L2's also . So if optimisedOracles of the Pool is Chainlink and if the sequencer outage happens , then it increases the chance of such issues happening.

Impact

Temporary Outage of important operations in UpdateWeightRunner & UpliftOnlyExample.sol .

The performUpdate is the one of the important functions in UpdateWeightRunner and the function could be out of use temporarily in such situations .

The getData function is also used in the addLiquidityProportional , onAfterRemoveLiquidity & afterUpdate functions of UpliftOnlyExample contract .

Tools Used

Manual Review

Recommendations

Add an access controlled function in UpdateWeightRunner to allow addition of new oracles to the Pool in case of emergencies .

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_removing_oracle_will_prevent_pools_to_update

Likelihood: Low, when an oracle is removed. Impact: High, Pools using the removed oracle will corrupt the gradient and moving average calculation.

Support

FAQs

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