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

Immutable `UpdateWeightRunner` in Rules Breaks Pool Weight Updates After Migration

huntoor

Summary

The UpdateRule contract uses an immutable address for updateWeightRunner, preventing new UpdateWeightRunner instances from interacting with existing rules after a pool migration, effectively breaking core weight update functionality.

Vulnerability Details

NOTE!: This issue assumed that issues regarding the need to recall setRuleForPool() during changing UpdateWeightRunner in QuantAMMWeightedPool has been solved

Rule contracts register UpdateWeightRunner as an immutable, but the problem is that its normal behavior to change UpdateWeightRunner since its a singleton design that my have hotfixes or upgrades here and there as said by DEVs in the comments here

File: IQuantAMMWeightedPool.sol
144: /// @notice this is to update the runner for the pool. This is for hotfixes and is timelock protected.
145: function setUpdateWeightRunnerAddress(address _updateWeightRunner) external;

Assuming that the pool register its settings, oracles and rules successfully

  • The new UpdateWeightRunner will fail to interact with the old rules in critical areas, those areas are:

1- Weight updates through performUpdate():

// UpdateWeightRunner.sol
function performUpdate(address _pool) {
_performUpdateAndGetData(_pool, settings);
// Calls CalculateNewWeights() on rule
}
​
// UpdateRule.sol
function CalculateNewWeights() {
require(msg.sender == updateWeightRunner, "UNAUTH_CALC"); // Fails for new runner
}

2- Manual intermediate value updates:

// UpdateWeightRunner.sol
function setIntermediateValuesManually() {
rule.initialisePoolRuleIntermediateValues();
}
​
// UpdateRule.sol
function initialisePoolRuleIntermediateValues() {
require(msg.sender == _poolAddress || msg.sender == updateWeightRunner, "UNAUTH");
}

Although in QuantAMMWeightedPool, it can update its UpdateWeightRunner:

function setUpdateWeightRunnerAddress(address _updateWeightRunner) {
require(msg.sender == quantammAdmin, "ONLYADMIN");
updateWeightRunner = UpdateWeightRunner(_updateWeightRunner);
}

The rules remain locked to the old runner due to immutable storage Leading to:

  • calls to UpdateWeightRunner::performUpdate() and UpdateWeightRunner::setIntermediateValuesManually() will always revert after migration

  • Forces redeployment of entire rules after any new UpdateWeightRunner deployment

Impact

  • calls to UpdateWeightRunner::performUpdate() and UpdateWeightRunner::setIntermediateValuesManually() will always revert after migration

  • Forces redeployment of entire rules after any new UpdateWeightRunner deployment

Tools Used

Manual Review

Recommendations

  • Remove immutable modifier in UpdateRule and add function to be able to change it

contract UpdateRule {
address public updateWeightRunner;
function setUpdateWeightRunner(address newRunner) external {
require(msg.sender == currentRunner, "UNAUTH");
updateWeightRunner = newRunner;
}
}
Updates

Lead Judging Commences

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

finding_setUpdateWeightRunnerAddress_will_DoS_rules

Likelihood: Low, when setting a new UpdateWeightRunner (for hotfixes) Impact: High, DoS performUpdate and force redeployment

Appeal created

huntoor Submitter
10 months ago
n0kto Lead Judge
10 months ago
n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!