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

Incorrect Loop Handling Due to Variable Initialization

kwakudr

Summary

In the MinimumVarianceUpdateRule contract's _getWeights function, there are loops that rely on a loop variable i. The issue arises because the loop variable i is not properly initialized before each loop. Instead, it is declared once at the function level and reused across multiple loops without resetting it to zero. This can lead to loops not executing as intended, resulting in incomplete calculations.

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/rules/MinimumVarianceUpdateRule.sol#L29C5-L76C14

Vulnerability Details

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/rules/MinimumVarianceUpdateRule.sol#L29C5-L76C14

function _getWeights(
int256[] calldata _prevWeights,
int256[] memory _data,
int256[][] calldata _parameters, //
QuantAMMPoolParameters memory _poolParameters
) internal override returns (int256[] memory newWeightsConverted) {
_poolParameters.numberOfAssets = _prevWeights.length;
​
​
//reuse of the newWeights array allows for saved gas in array initialisation
int256[] memory newWeights = _calculateQuantAMMVariance(_data, _poolParameters);
​
​
int256 divisionFactor;
newWeightsConverted = new int256[](_prevWeights.length);
​
​
if (_parameters[0].length == 1) {
int256 mixingVariance = _parameters[0][0];
// calculating (1 − Λ)*Σ^−1(t)
int256 oneMinusLambda = ONE - mixingVariance;
for (uint i; i < _prevWeights.length; ) {
int256 precision = ONE.div(newWeights[i]);
divisionFactor += precision;
newWeights[i] = oneMinusLambda.mul(precision);
unchecked {
++i;
}
}
// To avoid intermediate overflows (because of normalization), we only downcast in the end to an uint64
// Divide precision vector by the sum of precisions and add Λw(t - 1)
// (Λ * w(t − 1)) + ((1 − Λ)*Σ^−1(t)) / N,j=1∑ Σ^−1 j(t)
for (uint i; i < _prevWeights.length; ) {
int256 res = mixingVariance.mul(int256(_prevWeights[i])) + newWeights[i].div(divisionFactor);
newWeightsConverted[i] = res;
unchecked {
++i;
}
}
} else {
// calculating (1 − Λ)*Σ^−1(t)
for (uint i; i < _prevWeights.length; ) {
int256 mixingVariance = _parameters[0][i];
int256 oneMinusLambda = ONE - mixingVariance;
int256 precision = ONE.div(newWeights[i]);
divisionFactor += precision;
newWeights[i] = oneMinusLambda.mul(precision);
unchecked {
++i;
}
}

The loop variable i is declared once at the function-level scope: uint i;. It is used in multiple loops without being reinitialized between them. After the first loop finishes, i equals _prevWeights.length. The condition for the next loop i < _prevWeights.length is no longer satisfied, so the loop doesn't execute.

if (_parameters[0].length == 1) {
int256 mixingVariance = _parameters[0][0];
// calculating (1 − Λ)*Σ^−1(t)
int256 oneMinusLambda = ONE - mixingVariance;
for (uint i; i < _prevWeights.length; ) {
int256 precision = ONE.div(newWeights[i]);
divisionFactor += precision;
newWeights[i] = oneMinusLambda.mul(precision);
unchecked {
++i;
}

See the first loop above. Since i is declared at the function level and not reinitialized here, it starts with its current value. At the beginning of the first loop, i is 0 (since it's a new function call).

The loop runs as long as i < _prevWeights.length. i is incremented in each iteration via ++i. When i reaches _prevWeights.length, the loop exits.

After the first loop, i equals _prevWeights.length.

for (uint i; i < _prevWeights.length; ) {
int256 res = mixingVariance.mul(int256(_prevWeights[i])) + newWeights[i].div(divisionFactor);
newWeightsConverted[i] = res;
unchecked {
++i;
}
}
} else {
// calculating (1 − Λ)*Σ^−1(t)
for (uint i; i < _prevWeights.length; ) {
int256 mixingVariance = _parameters[0][i];
int256 oneMinusLambda = ONE - mixingVariance;
int256 precision = ONE.div(newWeights[i]);
divisionFactor += precision;
newWeights[i] = oneMinusLambda.mul(precision);
unchecked {
++i;
}
}

The second loop can be seen above. i is not reinitialized, so it remains at _prevWeights.length. The loop condition i < _prevWeights.length evaluates to false (since i == _prevWeights.length). The loop body is not executed at all. All calculations intended to be performed in the second loop are skipped.

The second loop is crucial as it calculates the final adjusted weights and stores them in newWeightsConverted.

Note: similar issue can be found in ChannelFollowingUpdateRule.sol::_getWeights. I don't want to Belabor the point with another report on the similar issue

Impact

The newWeightsConverted array remains uninitialized or contains default values (likely zeros).

The function returns incorrect weight values. The final weights are incorrect because the necessary calculations in the second loop are never performed

Tools Used

Manual Review

Recommendations

Initialize i Before Each Loop: Ensure that i starts at 0 for each loop by declaring it within the loop's scope.

Updates

Lead Judging Commences

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

Support

FAQs

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

Give us feedback!