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

Gradient miscalculation due to using an incorrect array index

crunter

Summary

In QuantammGradientBasedRule::_calculateQuantAMMGradient, when calculating the gradient for the case where lambda is not scalar, a wrong index is used to store the resulting data in the intermediateGradientStates array, making its contents unreliable.

Vulnerability Details

Depending on the specific rule parameters, there are several possible paths to follow to calculate the gradient in QuantammGradientBasedRule::_calculateQuantAMMGradient. In all cases, previous intermediate values are needed to perform the calculations. These values are stored in the intermediateGradientStates array. However, in the case where lambda is not scalar, an incorrect index is used to store the values, which corrupts the data and compromises subsequent calculations.

> QuantammGradientBasedRule.sol
​
function _calculateQuantAMMGradient(
int256[] memory _newData,
QuantAMMPoolParameters memory _poolParameters
) internal returns (int256[] memory) {
QuantAMMGradientLocals memory locals;
locals.finalValues = new int256[](_poolParameters.numberOfAssets);
​
// @audit - intermediateGradientStates[_poolParameters.pool] array content is used to calculate the new gradient
@> locals.intermediateGradientState = _quantAMMUnpack128Array(
@> intermediateGradientStates[_poolParameters.pool],
@> _poolParameters.numberOfAssets
);
​
// lots initialised before looping to save gas
bool notDivisibleByTwo = _poolParameters.numberOfAssets % 2 != 0;
uint numberOfAssetsMinusOne = _poolParameters.numberOfAssets - 1;
int256 convertedLambda = int256(_poolParameters.lambda[0]);
int256 oneMinusLambda = ONE - convertedLambda;
​
//You cannot have a one token pool so if its one element you know it's scalar
if (_poolParameters.lambda.length == 1) {
...
// @audit - Logic for the case when lambda is scalar
...
} else {
// @audit - Logic for the case when lambda is not scalar
​
// if the parameters are defined as per constituent we do the same as the if loop but
//tracking the appropriate lambda for each asset and the appropriate storage index
if (notDivisibleByTwo) {
--numberOfAssetsMinusOne;
}
​
for (uint i; i < numberOfAssetsMinusOne; ) {
unchecked {
convertedLambda = int256(_poolParameters.lambda[i]);
oneMinusLambda = ONE - convertedLambda;
locals.mulFactor = oneMinusLambda.pow(THREE).div(convertedLambda);
}
​
// a(t) = λa(t - 1) + (p(t) - p̅(t)) / (1 - λ)
locals.intermediateValue =
convertedLambda.mul(locals.intermediateGradientState[i]) +
(_newData[i] - _poolParameters.movingAverage[i]).div(oneMinusLambda);
​
locals.intermediateGradientState[i] = locals.intermediateValue;
locals.finalValues[i] = locals.mulFactor.mul(locals.intermediateValue);
​
unchecked {
locals.secondIndex = i + 1;
convertedLambda = int256(_poolParameters.lambda[locals.secondIndex]);
oneMinusLambda = ONE - convertedLambda;
locals.mulFactor = oneMinusLambda.pow(THREE).div(convertedLambda);
}
​
locals.secondIntermediateValue =
convertedLambda.mul(locals.intermediateGradientState[locals.secondIndex]) +
(_newData[locals.secondIndex] - _poolParameters.movingAverage[locals.secondIndex]).div(
oneMinusLambda
);
​
locals.finalValues[locals.secondIndex] = locals.mulFactor.mul(locals.secondIntermediateValue);
​
// @audit - index 'i' should be only used in the locals arrays. For the system global
// intermediateGradientStates[_poolParameters.pool] array, the correct index is 'locals.storageArrayIndex'
// This indexing issue causes the data to be stored in the wrong slot
@> intermediateGradientStates[_poolParameters.pool][i] = _quantAMMPackTwo128(
locals.intermediateGradientState[i],
locals.secondIntermediateValue
);
unchecked {
i += 2;
++locals.storageArrayIndex;
}
}
​
//take care of the potential sticky end if not divisible by two
if (notDivisibleByTwo) {
unchecked {
++numberOfAssetsMinusOne;
convertedLambda = int256(_poolParameters.lambda[numberOfAssetsMinusOne]);
oneMinusLambda = ONE - convertedLambda;
locals.mulFactor = oneMinusLambda.pow(THREE).div(convertedLambda);
}
​
locals.intermediateValue =
convertedLambda.mul(locals.intermediateGradientState[numberOfAssetsMinusOne]) +
(_newData[numberOfAssetsMinusOne] - _poolParameters.movingAverage[numberOfAssetsMinusOne]).div(
oneMinusLambda
);
​
locals.finalValues[numberOfAssetsMinusOne] = locals.mulFactor.mul(locals.intermediateValue);
​
// @audit - Here, the correct index is used.
@> intermediateGradientStates[_poolParameters.pool][locals.storageArrayIndex] = locals.intermediateValue;
}
}
​
return locals.finalValues;
}
​

Impact

Impact: High

Gradient calculation plays a fundamental role, as almost all rules depend on it to work correctly. If the gradient is incorrect, the result of subsequent calculations is unreliable.

Likelihood: High

Tools Used

Manual Review

Recommendations

It is very important to use the correct index to store the data in the corresponding slot.

> QuantammGradientBasedRule.sol
​
function _calculateQuantAMMGradient(
int256[] memory _newData,
QuantAMMPoolParameters memory _poolParameters
) internal returns (int256[] memory) {
QuantAMMGradientLocals memory locals;
locals.finalValues = new int256[](_poolParameters.numberOfAssets);
​
// @audit - intermediateGradientStates[_poolParameters.pool] array content is used to calculate the new gradient
locals.intermediateGradientState = _quantAMMUnpack128Array(
intermediateGradientStates[_poolParameters.pool],
_poolParameters.numberOfAssets
);
​
// lots initialised before looping to save gas
bool notDivisibleByTwo = _poolParameters.numberOfAssets % 2 != 0;
uint numberOfAssetsMinusOne = _poolParameters.numberOfAssets - 1;
int256 convertedLambda = int256(_poolParameters.lambda[0]);
int256 oneMinusLambda = ONE - convertedLambda;
​
//You cannot have a one token pool so if its one element you know it's scalar
if (_poolParameters.lambda.length == 1) {
...
// @audit - Logic for the case when lambda is scalar
...
} else {
// @audit - Logic for the case when lambda is not scalar
​
// if the parameters are defined as per constituent we do the same as the if loop but
//tracking the appropriate lambda for each asset and the appropriate storage index
if (notDivisibleByTwo) {
--numberOfAssetsMinusOne;
}
​
for (uint i; i < numberOfAssetsMinusOne; ) {
unchecked {
convertedLambda = int256(_poolParameters.lambda[i]);
oneMinusLambda = ONE - convertedLambda;
locals.mulFactor = oneMinusLambda.pow(THREE).div(convertedLambda);
}
​
// a(t) = λa(t - 1) + (p(t) - p̅(t)) / (1 - λ)
locals.intermediateValue =
convertedLambda.mul(locals.intermediateGradientState[i]) +
(_newData[i] - _poolParameters.movingAverage[i]).div(oneMinusLambda);
​
locals.intermediateGradientState[i] = locals.intermediateValue;
locals.finalValues[i] = locals.mulFactor.mul(locals.intermediateValue);
​
unchecked {
locals.secondIndex = i + 1;
convertedLambda = int256(_poolParameters.lambda[locals.secondIndex]);
oneMinusLambda = ONE - convertedLambda;
locals.mulFactor = oneMinusLambda.pow(THREE).div(convertedLambda);
}
​
locals.secondIntermediateValue =
convertedLambda.mul(locals.intermediateGradientState[locals.secondIndex]) +
(_newData[locals.secondIndex] - _poolParameters.movingAverage[locals.secondIndex]).div(
oneMinusLambda
);
​
locals.finalValues[locals.secondIndex] = locals.mulFactor.mul(locals.secondIntermediateValue);
​
- intermediateGradientStates[_poolParameters.pool][i] = _quantAMMPackTwo128(
+ intermediateGradientStates[_poolParameters.pool][locals.storageArrayIndex] = _quantAMMPackTwo128(
locals.intermediateGradientState[i],
locals.secondIntermediateValue
);
unchecked {
i += 2;
++locals.storageArrayIndex;
}
}
​
//take care of the potential sticky end if not divisible by two
if (notDivisibleByTwo) {
unchecked {
++numberOfAssetsMinusOne;
convertedLambda = int256(_poolParameters.lambda[numberOfAssetsMinusOne]);
oneMinusLambda = ONE - convertedLambda;
locals.mulFactor = oneMinusLambda.pow(THREE).div(convertedLambda);
}
​
locals.intermediateValue =
convertedLambda.mul(locals.intermediateGradientState[numberOfAssetsMinusOne]) +
(_newData[numberOfAssetsMinusOne] - _poolParameters.movingAverage[numberOfAssetsMinusOne]).div(
oneMinusLambda
);
​
locals.finalValues[numberOfAssetsMinusOne] = locals.mulFactor.mul(locals.intermediateValue);
​
intermediateGradientStates[_poolParameters.pool][locals.storageArrayIndex] = locals.intermediateValue;
}
}
​
return locals.finalValues;
}
​
Updates

Lead Judging Commences

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

finding_gradient_rules_more_than_3_assets_and_1_lambda_will_DoS_the_update

Likelihood: Medium/High, assets>4, lambdas > 1. Impact: Medium/High, DoS update but pool works fine. Pool with 5 assets will use incorrect weights.

Support

FAQs

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

Give us feedback!