QuantAMM

QuantAMM
49,600 OP
View results
Submission Details
Severity: low
Invalid

Inadequate Validation in `_setInitialMovingAverages` Function

Summary

The _setInitialMovingAverages function of QuantammMathMovingAverage contract and lacks proper validation when setting initial moving averages. This could lead to incorrect initialization of moving averages if the provided data does not match the expected number of assets.

Vulnerability Details

The _setInitialMovingAverages function uses a conditional statement with a logical OR (||) operator to check two conditions:

  1. If movingAverageLength is 0, indicating that there are no existing moving averages for the specified pool.

  2. If the length of _initialMovingAverages matches _numberOfAssets.

function _setInitialMovingAverages(
address _poolAddress,
int256[] memory _initialMovingAverages,
uint _numberOfAssets
) internal {
uint movingAverageLength = movingAverages[_poolAddress].length;
>> if (movingAverageLength == 0 || _initialMovingAverages.length == _numberOfAssets) {
//should be during create pool
movingAverages[_poolAddress] = _quantAMMPack128Array(_initialMovingAverages);
} else {
revert("Invalid set moving avg");
}
}

Due to the nature of short-circuit evaluation in logical expressions, if the first condition (movingAverageLength == 0) evaluates to true, the second condition (_initialMovingAverages.length == _numberOfAssets) is never evaluated. This means that the function can proceed to set moving averages if there are no existing moving averages for the specified pool without validating whether the provided initial averages correspond to the expected number of assets.

Impact

The function may set moving averages with an incorrect number of values if _initialMovingAverages does not match _numberOfAssets if there are no existing moving averages for the specified pool. This can result in inconsistent or invalid state within the contract.

Tools Used

Manual Review

Recommendations

Ensure both conditions are independently validated. Use an && operator to enforce that both movingAverageLength == 0 and _initialMovingAverages.length == _numberOfAssets must be true before setting the moving averages.

function _setInitialMovingAverages(
address _poolAddress,
int256[] memory _initialMovingAverages,
uint _numberOfAssets
) internal {
uint movingAverageLength = movingAverages[_poolAddress].length;
- if (movingAverageLength == 0 || _initialMovingAverages.length == _numberOfAssets) {
+ if (movingAverageLength == 0 && _initialMovingAverages.length == _numberOfAssets || _initialMovingAverages.length == _numberOfAssets) {
//should be during create pool
movingAverages[_poolAddress] = _quantAMMPack128Array(_initialMovingAverages);
} else {
revert("Invalid set moving avg");
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas / Admin is trusted / Pool creation is trusted / User mistake / Suppositions

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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

Give us feedback!