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

The `UpdateWeightRunner.sol#performUpdate()` function can always revert under certain conditions.

rampageaudit

Title

The UpdateWeightRunner.sol#performUpdate() function can always revert under certain conditions.

Summary

The performUpdate() function in UpdateWeightRunner.sol can revert due to boundary cases where the calculated weights become negative. This issue arises from the _getWeights() function in various update rule contracts, which does not handle cases where weights can be less than zero, causing the protocol to freeze during weight updates.

Links to affected code

Vulnerability details

Finding description and impact

The AntimomentumUpdateRule.sol#_getWeights() function is implemented as follows:

function _getWeights(
int256[] calldata _prevWeights,
int256[] memory _data,
int256[][] calldata _parameters,
QuantAMMPoolParameters memory _poolParameters
) internal override returns (int256[] memory newWeightsConverted) {
QuantAMMAntiMomentumLocals memory locals;
locals.kappa = _parameters[0];
locals.useRawPrice = false;
​
// the second parameter determines if antimomentum should use the price or the average price as the denominator
// using the average price has shown greater performance and resilience due to greater smoothing
if (_parameters.length > 1) {
locals.useRawPrice = _parameters[1][0] == ONE;
}
​
_poolParameters.numberOfAssets = _prevWeights.length;
​
locals.newWeights = _calculateQuantAMMGradient(_data, _poolParameters);
​
for (locals.i = 0; locals.i < _prevWeights.length; ) {
locals.denominator = _poolParameters.movingAverage[locals.i];
if (locals.useRawPrice) {
locals.denominator = _data[locals.i];
}
​
//1/p(t) · ∂p(t)/∂t used in both the main part of the equation and normalisation so saved to save gas
// used of new weights array allows reuse and saved gas
locals.newWeights[locals.i] = ONE.div(locals.denominator).mul(int256(locals.newWeights[locals.i]));
if (locals.kappa.length == 1) {
locals.normalizationFactor += locals.newWeights[locals.i];
} else {
locals.normalizationFactor += (locals.newWeights[locals.i].mul(locals.kappa[locals.i]));
}
unchecked {
++locals.i;
}
}
​
// To avoid intermediate overflows (because of normalization), we only downcast in the end to an uint64
newWeightsConverted = new int256[](_prevWeights.length);
if (locals.kappa.length == 1) {
locals.normalizationFactor /= int256(_prevWeights.length);
// w(t − 1) + κ ·(ℓp(t) − 1/p(t) · ∂p(t)/∂t)
​
for (locals.i = 0; locals.i < _prevWeights.length; ) {
99-> int256 res = int256(_prevWeights[locals.i]) +
int256(locals.kappa[0]).mul(locals.normalizationFactor - locals.newWeights[locals.i]);
newWeightsConverted[locals.i] = res;
unchecked {
-> ++locals.i;
}
}
} else {
for (locals.i = 0; locals.i < locals.kappa.length; ) {
locals.sumKappa += locals.kappa[locals.i];
unchecked {
++locals.i;
}
}
​
locals.normalizationFactor = locals.normalizationFactor.div(locals.sumKappa);
for (locals.i = 0; locals.i < _prevWeights.length; ) {
// w(t − 1) + κ ·(ℓp(t) − 1/p(t) · ∂p(t)/∂t)
int256 res = int256(_prevWeights[locals.i]) +
119-> int256(locals.kappa[locals.i]).mul(locals.normalizationFactor - locals.newWeights[locals.i]);
-> require(res >= 0, "Invalid weight");
newWeightsConverted[locals.i] = res;
unchecked {
++locals.i;
}
}
}
​
return newWeightsConverted;
}

At line L119, the locals.normalizationFactor is a weighted average of locals.newWeights[locals.i] by kappa. Therefore, locals.normalizationFactor - locals.newWeights[locals.i] can be less than zero. In boundary cases, int256(locals.kappa[locals.i]).mul(locals.normalizationFactor - locals.newWeights[locals.i]) can result in res < 0, causing a guard rail failure. Consequently, UpdateWeightRunner.sol#performUpdate() will always revert in such cases. Additionally, there is no underflow check between lines L99~L104.

The UpdateRule.sol#L195~L203 code is as follows:

locals.unGuardedUpdatedWeights = _getWeights(_prevWeights, _data, _parameters, poolParameters);
​
//Guard weights is done in the base contract so regardless of the rule the logic will always be executed
updatedWeights = _guardQuantAMMWeights(
locals.unGuardedUpdatedWeights,
_prevWeights,
int128(uint128(_epsilonMax)),
int128(uint128(_absoluteWeightGuardRail))
);

As shown above, the protocol retrieves unguarded weights from the _getWeights() function and then guards the weights using clamping. There is no assumption that weights are always greater than zero. However, in some cases, the real implementation causes a revert due to underflow when kappa is provided as a vector instead of a scalar. When this boundary case occurs, the protocol is frozen for updating weights.

This issue also exists in ChannelFollowingUpdateRule.sol, DifferenceMomentumUpdateRule.sol, MomentumUpdateRule.sol, and PowerChannelUpdateRule.sol.

Recommended mitigation steps

The _getWeights() functions should be modified to prevent reversion when weights go below zero. For example, the AntimomentumUpdateRule.sol#_getWeights() function should be modified by removing the following line:

-- require(res >= 0, "Invalid weight");
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid_weights_can_be_negative_or_extreme_values

_clampWeights will check that these weights are positive and in the boundaries before writing them in storage.

Support

FAQs

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