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

Incorrect handling of `locals.notDivisibleByTwo` case in _calculateQuantAMMVariance` function

mbushbush

Incorrect handling of locals.notDivisibleByTwo case in _calculateQuantAMMVariance` function

Code Snippets

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/rules/base/QuantammVarianceBasedRule.sol#L51-L192

Summary

The _calculateQuantAMMVariance function fails to decrement locals.nMinusOne for pools with odd number assets adn vector lambda, causing an out-of-bounds error.

Vulnerability Details

The issue exists in the _calculateQuantAMMVariance function.

function _calculateQuantAMMVariance(
int256[] memory _newData,
QuantAMMPoolParameters memory _poolParameters
) internal returns (int256[] memory) {
QuantAMMVarianceLocals memory locals;
locals.n = _poolParameters.numberOfAssets;
locals.finalState = new int256[](locals.n);
locals.intermediateVarianceState = _quantAMMUnpack128Array(
intermediateVarianceStates[_poolParameters.pool],
locals.n
);
locals.nMinusOne = locals.n - 1;
locals.notDivisibleByTwo = locals.n % 2 != 0;
locals.convertedLambda = int256(_poolParameters.lambda[0]);
locals.oneMinusLambda = ONE - locals.convertedLambda;
​
//the packed int256 slot index to store the intermediate variance state
​
if (_poolParameters.lambda.length == 1) {
//scalar parameters mean the calculation is simplified and even if it increases function and
//contract size it decrease gas computed given iterative design tests
if (locals.notDivisibleByTwo) {
unchecked {
--locals.nMinusOne;
}
}
​
for (uint i; i < locals.nMinusOne; ) {
// Intermediate states are calculated in pairs to then SSTORE as we go along saving gas from a redundant SSTORE of length if we did the whole array
// calculating and storing in the same loop also saves loop costs
locals.intermediateState =
locals.convertedLambda.mul(locals.intermediateVarianceState[i]) +
(_newData[i] - _poolParameters.movingAverage[locals.n + i])
.mul(_newData[i] - _poolParameters.movingAverage[i])
.div(TENPOWEIGHTEEN); // p(t) - p̅(t - 1))_i * (p(t) - p̅(t))_i
​
locals.intermediateVarianceState[i] = locals.intermediateState;
locals.finalState[i] = locals.oneMinusLambda.mul(locals.intermediateState);
​
unchecked {
locals.secondIndex = i + 1;
}
​
locals.intermediateState =
locals.convertedLambda.mul(locals.intermediateVarianceState[locals.secondIndex]) +
(_newData[locals.secondIndex] - _poolParameters.movingAverage[locals.n + locals.secondIndex])
.mul(_newData[locals.secondIndex] - _poolParameters.movingAverage[locals.secondIndex])
.div(TENPOWEIGHTEEN); // p(t) - p̅(t - 1))_i * (p(t) - p̅(t))_i
​
locals.intermediateVarianceState[locals.secondIndex] = locals.intermediateState;
intermediateVarianceStates[_poolParameters.pool][locals.storageIndex] = _quantAMMPackTwo128(
locals.intermediateVarianceState[i],
locals.intermediateVarianceState[locals.secondIndex]
);
​
locals.finalState[locals.secondIndex] = locals.oneMinusLambda.mul(locals.intermediateState);
​
unchecked {
i += 2;
++locals.storageIndex;
}
}
​
if (locals.notDivisibleByTwo) {
unchecked {
++locals.nMinusOne;
}
locals.intermediateState =
locals.convertedLambda.mul(locals.intermediateVarianceState[locals.nMinusOne]) +
(_newData[locals.nMinusOne] - _poolParameters.movingAverage[locals.n + locals.nMinusOne])
.mul(_newData[locals.nMinusOne] - _poolParameters.movingAverage[locals.nMinusOne])
.div(TENPOWEIGHTEEN); // p(t) - p̅(t - 1))_i * (p(t) - p̅(t))_i
​
locals.intermediateVarianceState[locals.nMinusOne] = locals.intermediateState;
locals.finalState[locals.nMinusOne] = locals.oneMinusLambda.mul(locals.intermediateState);
intermediateVarianceStates[_poolParameters.pool][locals.storageIndex] = locals
.intermediateVarianceState[locals.nMinusOne];
}
} else {
//vector parameter calculation is the same but we have to keep track of and access the right vector parameter
for (uint i; i < locals.nMinusOne; ) {
unchecked {
locals.convertedLambda = int256(_poolParameters.lambda[i]);
locals.oneMinusLambda = ONE - locals.convertedLambda;
}
// p(t) - p̅(t - 1))_i * (p(t) - p̅(t))_i
locals.intermediateState =
locals.convertedLambda.mul(locals.intermediateVarianceState[i]) +
(_newData[i] - _poolParameters.movingAverage[locals.n + i])
.mul(_newData[i] - _poolParameters.movingAverage[i])
.div(TENPOWEIGHTEEN);
​
locals.intermediateVarianceState[i] = locals.intermediateState;
locals.finalState[i] = locals.oneMinusLambda.mul(locals.intermediateState);
​
unchecked {
locals.secondIndex = i + 1;
locals.convertedLambda = int256(_poolParameters.lambda[i + 1]);
locals.oneMinusLambda = ONE - locals.convertedLambda;
}
// p(t) - p̅(t - 1))_i * (p(t) - p̅(t))_i
locals.intermediateState =
locals.convertedLambda.mul(locals.intermediateVarianceState[locals.secondIndex]) +
(_newData[locals.secondIndex] - _poolParameters.movingAverage[locals.n + locals.secondIndex])
.mul(_newData[locals.secondIndex] - _poolParameters.movingAverage[locals.secondIndex])
.div(TENPOWEIGHTEEN);
​
locals.intermediateVarianceState[locals.secondIndex] = locals.intermediateState;
​
intermediateVarianceStates[_poolParameters.pool][locals.storageIndex] = _quantAMMPackTwo128(
locals.intermediateVarianceState[i],
locals.intermediateVarianceState[locals.secondIndex]
);
locals.finalState[locals.secondIndex] = locals.oneMinusLambda.mul(locals.intermediateState);
​
unchecked {
i += 2;
++locals.storageIndex;
}
}
​
if (locals.notDivisibleByTwo) {
unchecked {
++locals.nMinusOne;
locals.convertedLambda = int256(_poolParameters.lambda[locals.nMinusOne]);
locals.oneMinusLambda = ONE - locals.convertedLambda;
}
locals.intermediateState =
locals.convertedLambda.mul(locals.intermediateVarianceState[locals.nMinusOne]) +
(_newData[locals.nMinusOne] - _poolParameters.movingAverage[locals.n + locals.nMinusOne])
.mul(_newData[locals.nMinusOne] - _poolParameters.movingAverage[locals.nMinusOne])
.div(TENPOWEIGHTEEN); // p(t) - p̅(t - 1))_i * (p(t) - p̅(t))_i
​
locals.intermediateVarianceState[locals.nMinusOne] = locals.intermediateState;
locals.finalState[locals.nMinusOne] = locals.oneMinusLambda.mul(locals.intermediateState);
intermediateVarianceStates[_poolParameters.pool][locals.storageIndex] = locals
.intermediateVarianceState[locals.nMinusOne];
}
}
​
return locals.finalState;
}

For scalar lambda, locals.nMinusOne is decremented correctly for odd-asset pools. However, for vector lambda, this decrement is missing, causing an out-of-bounds error when accessing _poolParameters.lambda[locals.nMinusOne].

Impact

The missing decrement for vector lambda causes an out-of-bounds error, leading to a Denial of Service (DoS) that disrupts operations like CalculateNewWeights in odd-asset pools.

Tools Used

Manual Review

Recommendations

Decrease locals.nMinusOne by 1 in case the pool has odd number of assets and lambda is a vector.

Updates

Lead Judging Commences

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

finding_calculateQuantAMMVariance_revert_when_vector_lambda_and_odd_asset_number

Likelihood: Medium/High, odd asset number + lambda is a vector. Impact: Medium/High, DoS the update.

Support

FAQs

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