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

Unchecked targetArrayIndex Increment Can Corrupt Adjacent Matrix Storage Slots

cheatc0d33

Description

A vulnerability exists in the VectorRuleQuantAMMStorage contract where the _quantAMMPack128Matrix function performs unsafe storage writes that could corrupt arbitrary contract state. Here is the vulnerable code section:

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

// In VectorRuleQuantAMMStorage contract
function _quantAMMPack128Matrix(int256[][] memory _sourceMatrix, int256[] storage _targetArray) internal {
uint targetArrayIndex;
// ... other variables
unchecked {
// ... loop logic
if (right == 1) {
right = 0;
_targetArray[targetArrayIndex] = (leftInt << 128) | // Vulnerable: no bounds check
int256(uint256(_sourceMatrix[i][j] << 128) >> 128);
++targetArrayIndex; // Can overflow
}
// ... more loop logic
}
}

This vulnerability is particularly dangerous because the function is called directly from QuantAMMCovarianceBasedRule's core logic:

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/rules/base/QuantammCovarianceBasedRule.sol#L132

// In QuantAMMCovarianceBasedRule contract
function _setIntermediateCovariance(address _poolAddress, int256[][] memory _initialValues, uint _numberOfAssets) internal {
// ... validation logic
// Vulnerable call that could corrupt storage
_quantAMMPack128Matrix(_initialValues, intermediateCovarianceStates[_poolAddress]);
}

The issue is severe because:

  1. The function performs storage writes using an unchecked index

  2. Storage layout in Solidity is sequential, meaning overflow could corrupt:

    • Other state variables in VectorRuleQuantAMMStorage

    • State in inherited contracts like QuantAMMCovarianceBasedRule

    • Critical AMM parameters like pool weights and balances

Example exploitation scenario:

// Contract storage layout
slot[0]: contract owner
slot[1]: first matrix elements
slot[2]: second matrix elements
// ... if targetArrayIndex overflows, could overwrite slot[0]

Impact

The vulnerability in array bounds checking affects the integrity of the QuantAMM's storage system. While the initial length check provides some protection, subsequent unchecked increments of targetArrayIndex within the matrix packing loop could still result in out-of-bounds writes. The primary risk lies in corruption of the packed covariance data itself, as the storage slots adjacent to the matrix array likely contain other critical pool parameters. If targetArrayIndex exceeds bounds, it would overwrite these adjacent parameters, potentially corrupting pool weights, fee calculations, or other AMM state variables. This would cause the pool to operate with incorrect parameters until the corruption is detected, though the likelihood of bypassing the initial length check is relatively low.

Recommended Mitigation Steps

  1. Add explicit bounds checking before storage writes:

function *quantAMMPack128Matrix(int256[][] memory *sourceMatrix, int256[] storage _targetArray) internal {
uint256 targetArrayIndex = 0;
uint256 targetLength = _targetArray.length;
for (uint i; i < _sourceMatrix.length; ) {
for (uint j; j < _sourceMatrix[i].length; ) {
if (right == 1) {
// Add bounds check
require(targetArrayIndex < targetLength, "Array index out of bounds");
right = 0;
_targetArray[targetArrayIndex] = (leftInt << 128) |
int256(uint256(_sourceMatrix[i][j] << 128) >> 128);
unchecked { ++targetArrayIndex; }
}
// ... rest of the logic
}
}
}

  1. Remove the unchecked block from critical operations:

// Keep unchecked only for simple incrementors
unchecked { ++i; ++j; }
// Remove unchecked from storage operations
if (right == 1) {
require(targetArrayIndex < _targetArray.length, "Index out of bounds");
// ... storage write operations
}
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.