QuantAMM

Summary

In function UpdateRule::CalculateNewWeights(), it is possible for the sender to pass a previousWeights array with length less than that of pool's movingAverages length leading to array out-of-bound exception.

While in function initialisePoolRuleIntermediateValues, moving averages is initially set by calling function _setInitialMovingAverages but there is no limit on how many values that are allowed to set, this means that, the movingAverage state variable for a pool, can be set to an array any length.

Vulnerability Details

In the smart contract of _quantAMMUnpack128Array() function, it requires that _sourceArray.length is at least twice longer than the targetArray length. Thus:

require(_sourceArray.length * 2 >= _targetArrayLength, "SRC!=TGT");

but there is an assumption that the lengths are always equal but that might not always be the case since movingAverages of a pool can be set to an array of variable lenth. In an instance where _targetArrayLength is less than sourceArray.length, this would lead to array out of bound exception. This is possible because of the block of code in _quantAMMUnpack128Array function where the loop is run as many times the lenth of _sourceArray.length rather than that of targetArrayLength the targetIndex tries to access targetArray index that is out of bound. UpdateRule::CalculateNewWeights() internally calls _quantAMMUnpack128Array(). Here's the code that produces array out of bound error:

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/QuantAMMStorage.sol#L344

Also, in the case where !divisibleByTwo = true, but with targetArray length less than _sourceArray.lenth, the lines of code below will lead to targetArray having unexpexted data because the last element of the _sourceArray is less than the targetArray, the last element of targetArray is overwritten with the last element of the sourceArray (sourceArrayLengthMinusOne) this means that all elements from the last index of the targetArray and last index of the sourceArray would be lost leading to unexpected values return from the function _quantAMMUnpack128Array. Here's the lines of code that produces this error:

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/QuantAMMStorage.sol#L358

Proof of Concept

Paste the test below in UpdateRule.t.sol:

Impact

The issue below can lead to DoS attack and possibly unexpected return data in UpdateRule::CalculateNewWeights() and other functions that calls _quantAMMUnpack128Array() internally.

Tools Used

Manual Review

Recommendations

To fix this issue, require that both sourceArray.length * 2 and targetArrayLength are strictly of the same length by replacing the code on line:
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/QuantAMMStorage.sol#L339

with this code:

require(_sourceArray.length * 2 == _targetArrayLength, "SRC!=TGT");