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

If the primary oracle reverts, then the backup oracles will not be called and critical operations will go offline

brgltd

Summary

Due to the lack of a try-catch in the primary oracle call, the system will not call the backup oracles and can experience a DDos if the primary oracle reverts.

Vulnerability Details

QuantAMM has a primary oracle plus a set of backup oracles. If the response from the primary oracle is stale, the system will proceed by attempting to retrieve responses from the backup oracles.

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L360-L365

However, a reasonable assumption would be that the primary oracle call can revert (momentarily or permanently). For example chainlink could deprecated certain oracles in the future and set them to return price equal to zero, which would correctly revert in ChainlinkOracle.

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-quantamm/contracts/ChainlinkOracle.sol#L30

Since the system contains these sets of backup oracles, the backups should be called when the primary oracle reverts.

The following POC shows that if the primary oracle reverts, the backup oracles will not be called and the TX will revert.

To run the POC, we'll add a failing oracle that reverts when calling on _getData().

1: Simulate an oracle that will revert when retrieving data.

Path: pkg/pool-quantamm/contracts/mock/MockFailingOracle.sol

// SPDX-License-Identifier: MIT
pragma solidity >=0.8.24;
​
import "@balancer-labs/v3-interfaces/contracts/pool-quantamm/OracleWrapper.sol";
​
contract MockFailingOracle is OracleWrapper {
int216 private fixedReply;
uint private immutable delay;
uint40 public oracleTimestamp;
​
bool shouldRevert = true;
​
constructor(int216 _fixedReply, uint _delay) {
fixedReply = _fixedReply;
delay = _delay;
oracleTimestamp = uint40(block.timestamp);
}
​
function updateData(int216 _fixedReply, uint40 _timestamp) public {
fixedReply = _fixedReply;
oracleTimestamp = _timestamp;
}
​
function _getData() internal view override returns (int216 data, uint40 timestamp) {
// Primary oracle is currently offline or permanently deprecated.
if (shouldRevert) {
revert("primary oracle is offline");
}
data = fixedReply;
timestamp = uint40(block.timestamp - delay);
}
}

2: Add the failing oracle on pkg/pool-quantamm/test/foundry/UpdateWeightRunner.t.sol.

Path: pkg/pool-quantamm/test/foundry/UpdateWeightRunner.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
// ...
​
import "../../contracts/mock/MockFailingOracle.sol";
​
// ...
​
contract UpdateWeightRunnerTest is Test, QuantAMMTestUtils {
// ...
​
function deployFailingOracle(int216 fixedValue, uint delay) internal returns (MockFailingOracle) {
MockFailingOracle oracle = new MockFailingOracle(fixedValue, delay);
return oracle;
}
​
// ...
}

3: Final POC function to demonstrate a primary oracle revert will result in full TX revert:

Path: pkg/pool-quantamm/test/foundry/UpdateWeightRunner.t.sol

function testPrimaryOracleIsOfflineBackupsAreNotCalled() public {
vm.startPrank(owner);
updateWeightRunner.setApprovedActionsForPool(address(mockPool), 3);
​
// Assume primaryOracle is offline, and backupOracle1/backupOracle2 are working.
MockFailingOracle primaryOracle = deployFailingOracle(1000, 0);
MockChainlinkOracle backupOracle1 = deployOracle(1001, 0);
MockChainlinkOracle backupOracle2 = deployOracle(1002, 0);
​
updateWeightRunner.addOracle(primaryOracle);
updateWeightRunner.addOracle(backupOracle1);
updateWeightRunner.addOracle(backupOracle2);
​
mockRule = new MockIdentityRule();
​
vm.stopPrank();
​
vm.startPrank(address(mockPool));
address[][] memory oracles = new address[][]();
oracles[0] = new address[]();
oracles[0][0] = address(primaryOracle);
oracles[1] = new address[]();
oracles[1][0] = address(backupOracle1);
oracles[2] = new address[]();
oracles[2][0] = address(backupOracle2);
​
uint64[] memory lambda = new uint64[]();
lambda[0] = 0.0000000005e18;
updateWeightRunner.setRuleForPool(
IQuantAMMWeightedPool.PoolSettings({
assets: new IERC20[](0),
rule: IUpdateRule(mockRule),
oracles: oracles,
updateInterval: 1,
lambda: lambda,
epsilonMax: 0.2e18,
absoluteWeightGuardRail: 0.2e18,
maxTradeSizeRatio: 0.2e18,
ruleParameters: new int256[][](),
poolManager: addr2
})
);
vm.stopPrank();
​
vm.warp(block.timestamp + UPDATE_INTERVAL);
​
// Currently it will revert.
// Ideally it would switch to the backup oracles if the primary oracle reverts.
vm.expectRevert("primary oracle is offline");
updateWeightRunner.performUpdate(address(mockPool));
}

Impact

In the worst case scenario a primary oracle gets deprecated permanently while the backup oracles continue to work without issues.

In this case, adding liquidity, removing liquidity and transfering LP tokens on UpliftOnlyExample would revert and be unavailable.

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

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

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

The variable _updateWeightRunner is immutable in UpliftOnlyExample and the oracle data of UpdateWeightRunner can be added only while the pool rule is not initialized, presumably it will be called by QuantAMMWeightedPool.initialize() -> QuantAmmWeightedPool._setRule(). This means UpliftOnlyExample would need a migration or re-deployment to use an UpdateWeightRunner where the price retrieval works.

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

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L236

Tools Used

Manual review.

Recommendations

Consider adding a try-catch on the primary oracle call. If there's more than 1 backup oracle, a try-catch could also be added on the backup calls with the exception of the last one. With this approach, the price retrieval would only revert if all oracles revert.

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.

Give us feedback!