Submission Details
Severity:
Variance calculation revert due to a missing locals.notDivisibleByTwo flag checking
Summary
When calculating the variance in QuantammVarianceBasedRule::_calculateQuantAMMVariance, for the case when lambda is not scalar, there is not check of the locals.notDivisibleByTwo flag, affecting further calculations and causing the process to revert.
Vulnerability Details
As a general rule, it is necessary to check whether the number of assets is divisible by two in order to perform the calculations correctly. When calculating the variance in QuantammVarianceBasedRule::_calculateQuantAMMVariance, such check is used to properly handle array indexing for retrieving and storing data. However, for the case when lambda is not scalar, the check of the locals.notDivisibleByTwo flag is missing, resulting in wrong calculations and even causing the process to revert with an array out-of-bounds access error.
> QuantammVarianceBasedRule.sol
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) {
// @audit - check of the flag to see if the number of assets is even
@> if (locals.notDivisibleByTwo) {
@> unchecked {
@> --locals.nMinusOne;
}
}
...
// @audit - logic for the case when lambda is scalar
...
} else {
// @audit - logic for the case when lambda is not scalar
@> // @audit - check of the locals.notDivisibleByTwo flag is missing, but further code assumes the checking was done.
//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 {
// @audit - when locals.notDivisibleByTwo flag is true, in the last iteration this index will point beyond the last element of the array
@> 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 {
// @audit - this index will point beyond the last element of the array
@> ++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;
}
Impact
Impact: High
Likelihood: Medium
Tools Used
Manual Review
Recommendations
Include the locals.notDivisibleByTwo flag checking before start the calculations when lambda is not scalar.
> QuantammVarianceBasedRule.sol
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) {
// @audit - check of the flag to see if the number of assets is even
if (locals.notDivisibleByTwo) {
unchecked {
--locals.nMinusOne;
}
}
...
// @audit - logic for the case when lambda is scalar
...
} else {
// @audit - logic for the case when lambda is not scalar
+ if (locals.notDivisibleByTwo) {
+ unchecked {
+ --locals.nMinusOne;
+ }
+ }
//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;
}
Updates
Lead Judging Commences
n0kto 10 months ago
Submission Judgement Published 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.
Prize pool breakdown
Total prize
49,600 OP
nSLOC
3,00017
OP / LOC
44,640
4,960
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.