QuantAMM

Summary

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

In the _quantAMMUnpack128Array(...) function, the loop’s logic for incrementing targetIndex and handling the “last odd element” introduces clear risks of out-of-bounds writes or redundant overwrites.
The root cause is that the require(_sourceArray.length * 2 >= _targetArrayLength, "SRC!=TGT"); check ensures sufficient bits but does not ensure safe writes. The loop must include boundary checks or partial reads of _sourceArray[i] based on the remaining space in targetArray.
Without these checks, as demonstrated in the example above, the loop will attempt to write beyond the bounds of the target array on the second iteration

Vulnerability Details

The issue lies primarily in how the targetIndex is handled within the loop. Specifically, the loop “attempts” to extract two 128-bit segments from _sourceArray[i] in each iteration, writing them to targetArray[targetIndex] and targetArray[targetIndex + 1]. However, at the end of each iteration, both i and targetIndex are unconditionally incremented (++i; ++targetIndex;).

Key Issues
1. No Boundary Check for targetIndex in the Loop
If _sourceArray.length * 2 (the number of 128-bit segments available) is just one more than _targetArrayLength, or if the number of 128-bit segments does not align perfectly, the loop will write beyond the bounds of targetArray (i.e., to targetIndex == _targetArrayLength or beyond). Although the function begins with the following requirement:

This only ensures there are enough bits, but it does not prevent the loop from writing out of bounds if no additional checks are made to stop the writes once the array is full.

1. Confusing Logic for the Last (Odd) Element

When the target array length is odd (!divisibleByTwo), the loop attempts to extract a pair of 128-bit segments (upper and lower) in each iteration. If the last _sourceArray[i] is reached, it may skip or over-read due to this pairing logic. At the end of the loop, the following code is executed:

This could result in situations where the loop already wrote to targetIndex (incremented beyond bounds) and then overwrites or writes again to an already full array.

A Simple Test Case

Consider the following scenario:
• _sourceArray.length = 2 (containing 2 int256 values, theoretically enough to provide 4 segments of 128 bits).
• _targetArrayLength = 3 (requiring 3 128-bit segments).

This input satisfies the requirement 2 * 2 >= 3 and passes the require check. However, during the loop:
1. First Iteration (i = 0)
• _sourceArray[0]’s upper 128 bits are written to targetArray[0].
• _sourceArray[0]’s lower 128 bits are written to targetArray[1] because the condition (!divisibleByTwo && i < sourceArrayLengthMinusOne) is satisfied.
• i++ becomes 1, and targetIndex++ becomes 2.
2. Second Iteration (i = 1)
• _sourceArray[1]’s upper 128 bits are written to targetArray[2].
• At the end of this iteration, targetIndex++ becomes 3, exceeding the bounds of targetArray (which has indices 0, 1, 2).
3. Final Step
Since !divisibleByTwo, the following code is executed:

This attempts to write to targetArray[2] again. However, targetIndex was already incremented to 3 during the loop, meaning the logic effectively overwrites or exceeds bounds depending on how Solidity handles array indexing. In strict EVM semantics, this is a classic out-of-bounds write (and Solidity would panic).

Root Cause

The primary issue is that the loop does not properly account for whether the target array has been fully written. Specifically:
• The unconditional increment of targetIndex during each iteration ignores whether the array has reached its limit.
•The “sticky end” logic for the last odd element introduces unnecessary complexity, leading to potential overwrites or out-of-bounds writes.

Impact

Tools Used

Recommendations