QuantAMM

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

Incorrect index used for the last asset in the pools with odd number of assets in QuantammVarianceBasedRule contract.

Summary

In the QuantammVarianceBasedRule contract, the ‘for’ loop length specified under the case of vector lambda is one more than required. This is causing issues in the index of the last token in case of odd number of assets in the pool which is resulting in the wrong values of (i) the new intermediate state for the variance update for the last token and (ii) the new variances vector for the pool.

Vulnerability Details

In the function _calculateQuantAMMVariance of the QuantammVarianceBasedRule contract, when the _poolParameters.lambda is a vector, there is an error in the length of the ‘for’ loop and is specified as one more than what is actually needed. Even though the number of times the ‘for’ loop is executed is correct in the present case also, the issue lies with the variable used ‘nMinusOne’ to specify the length of the ‘for’ loop.

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

} 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;
}

In the case of the pools with odd number of assets, after coming out the aforementioned ‘for’ loop, this variable ‘nMinusOne’ is incremented which causes the issue. In the scenario, the contract tries to retrieve the variables of non-existent (n + 1)th asset and its associated pool Parameters. Computes the required parameters using non-existent things and stores promptly for the designated ‘n’th asset. This leads in the wrong values of (i) the new intermediate state for the variance update for the last token and (ii) the new variances vector for the pool.

Impact

Very High

For the cases where the pool has odd number of assets, the problem is very severe as explained earlier. The pool will have wrong variance vector resulting in the failure of the QuantAMM rules.
For a pool with 5 assets, the expected storage for the intermediate state for the variance and of the variance vector are
[0], [1], [2] for the intermediate state, and
[0], [1], [2], [3], [4] for the variance vector

The function calculateQuantAMMVariance stores as
[0], [1], [
], [3] for the intermediate state, and
[0], [1], [2], [3], [_], [5] for the variance vector.

Thus, the last intermediate state and the last element of the variance vector do not have any relation with any of the assets in the pool.

Tools Used

Manual review

Recommended Mitigation

The following portion of the code

if (locals.notDivisibleByTwo) {
unchecked {
++locals.nMinusOne;
}

needs to be added after line number 129 resulting in the following

} else {
if (locals.notDivisibleByTwo) { // @audit added
unchecked { // @audit added
++locals.nMinusOne; // @audit added
} // @audit added
//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;
}
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.