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

Moving Average Length Validation Prevents Admin Override for Rules Requiring Historical Data

0xhuntoor

Summary

The initialisePoolRuleIntermediateValues() function in UpdateRule fails to properly handle rules like MinimumVarianceUpdateRule that require double the number of moving averages (current + previous) due to a length validation check in _setInitialMovingAverages().

Vulnerability Details

The issue occurs in the following call flow:

// UpdateWeightRunner.sol
function setIntermediateValuesManually(
address _poolAddress,
int256[] memory _newMovingAverages,
int256[] memory _newParameters,
uint _numberOfAssets
) external {
// ... auth checks ...
rule.initialisePoolRuleIntermediateValues(_poolAddress, _newMovingAverages, _newParameters, _numberOfAssets);
}
// QuantammMathMovingAverage.sol
function _setInitialMovingAverages() internal {
if (movingAverageLength == 0 || _initialMovingAverages.length == _numberOfAssets) {
movingAverages[_poolAddress] = _quantAMMPack128Array(_initialMovingAverages);
} else {
revert("Invalid set moving avg");
}
}

The MinimumVarianceUpdateRule requires double the assets count for moving averages:

uint16 private constant REQUIRES_PREV_MAVG = 1;

And this how storage should look for that rule by DEV comment

// QuantAMMMathMovingAverage.sol
// this can be just the moving averages per token, or if prev moving average is true then it is [...moving averages, ...prev moving averages]
​
mapping(address => int256[]) public movingAverages;

The validation check _initialMovingAverages.length == _numberOfAssets prevents setting both current and previous moving averages, only allowing current ones to be set.

This will leave two options to the admin:

  1. Not using the function

  2. Use the function setting only moving average of the length of number of assets

If its the first one, then the availability of function is lost

If its the second option, then other integrators of that stored previous moving averages will be cornuted

Impact

  • Admin cannot properly set/override moving averages for rules requiring previous values

  • Loss of historical moving average data when manual override is needed

  • Breaks integrations from other protocols relying on both current and previous moving average values

Tools Used

Manual review

Recommendations

Modify the validation in _setInitialMovingAverages() to handle rules requiring different moving average lengths or there can be a check to assure that new values are the same length as movingAverageLength retrieved from the mapping

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

0xhuntoor Submitter
10 months ago
0xhuntoor Submitter
10 months ago
0xhuntoor Submitter
10 months ago
n0kto Lead Judge
9 months ago
n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_setInitialMovingAverages_does_not_work_with_prevMovAverage

Likelihood: Low, admin needs to use the breakglass function on a rule with prevMovAverage set to true. Impact: Medium/High, DoS that breakglass function.

Support

FAQs

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