QuantAMM

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

Insufficient checks on the target array

Summary

In the QuantAMMStorage contract the checks carried out on the target array length are insufficient as it can result in the elements not being stored in contiguous positions in the target array for the cases where the input square matrix has odd number of elements and the target array length specified is more than that necessary.

Vulnerability Details

Refer to the internal function _quantAMMPack128Matrix in the QuantAMMStorage contract. This function is used to pack n 128 bit integers (int256[][] memory _sourceMatrix) into n/2 (or (n + 1)/2 if n is odd) 256 bit integers (int256[] storage _targetArray). It is known that the _sourceMatrix is always a square matrix.

In the function, the only check done on the length of the target array is

require(targetArrayLength * 2 >= _sourceMatrix.length * _sourceMatrix.length, "Matrix doesnt fit storage");

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

This check is not sufficient to take care of all conditions. The presence of this check itself implies that the targetArrayLength can be different from the one actually required.

Take, for instance, any square matrix with odd number of elements (i.e. n x n matrix where ‘n’ is an odd number). In this case, the ((n * n) – 1) becomes an even number. Thus, these ((n * n) – 1) elements of the matrix will be stored in the (((n * n) – 1) /2) consecutive locations of the array, as per the code in the double ‘for’ loops, with the array index incremented within these loops. Refer :
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/QuantAMMStorage.sol#L391-L413

However, the last element of this square matrix with odd number of elements is stored separately without any reference to the array index used within the double ‘for’ loops. Instead, it uses the length of the target array passed to this function. So, if the length of the target array passed to this function is more than required, it will result in the last element being stored at the end of the target array, leaving the additional array elements of the target array not being specified. Refer:
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/QuantAMMStorage.sol#L414-L418

Proof of Concept

Consider the _sourceMatrix passed as a 3 x 3 matrix of 128 bit integers arranged as

| |1|, |2|, |3|, |
| |4|, |5|, |6|, |
| |7|, |8|, |9| |

and the _targetArray passed as a 7 element array. Note that this size _targetArray meets the requirement of the codebase.
After executing the function _quantAMMPack128Matrix, the elements in the target array will be arranged as

| 1 2 | 3 4 | 5 6 | 7 8 | _ _ | _ _ | _ 9 |

Whereas the desired format is
| 1 2 | 3 4 | 5 6 | 7 8 | _ 9 |

Thus, it is easily noticeable that the conditions/requirements imposed on the target array are not sufficient.

Impact

These are generally used for covariance computations and it is expected that the elements are stored in contiguous locations in the array. Any deviation will affect the covariance computations resulting in errors.

Tools Used

Manual review

Recommended Mitigation

The best way to mitigate the issue is to ensure that the target array length specified is neither smaller nor bigger than required. This can be done by adding one more require statement in the codebase on the length of the target array.

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

uint targetArrayLength = _targetArray.length;
require(targetArrayLength * 2 >= _sourceMatrix.length * _sourceMatrix.length, "Matrix doesnt fit storage");
uint targetArrayIndex;

can be modified to

uint targetArrayLength = _targetArray.length;
require(targetArrayLength * 2 >= _sourceMatrix.length * _sourceMatrix.length, "Matrix doesnt fit storage");
require((targetArrayLength – 1) * 2 < _sourceMatrix.length * _sourceMatrix.length, "Extra storage for matrix");
uint targetArrayIndex;

The additional require statement is as much necessary as the existing require statement in the codebase. If any abnormal target array length is passed to the function, the function must revert.

The other option is to use the target array index from inside the double ‘for’ loops to store the last element of the square matrix with odd number of elements. However, this will leave the following elements of the target array empty (for the cases where the target array size is larger than necessary). Though the required elements are in contiguous position in the target array, it has some elements in the array which are not filled.

The first option is better choice.

Updates

Lead Judging Commences

n0kto Lead Judge 7 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.