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

Array Management Flaw in Position Removal Causes Irreversible Position Data Corruption

cheatc0d33

Description

The onAfterRemoveLiquidity function in UpliftOnlyExample contains a critical flaw in its array management logic when processing position removals. The function attempts to maintain a FILO (First-In-Last-Out) order while removing positions, but incorrectly combines array deletion and pop operations:

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L471

for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {
if (feeDataArray[i].amount <= localData.amountLeft) {
// ... fee calculations ...
delete feeDataArray[i];
feeDataArray.pop(); // Incorrectly removes last element while processing from end
}
}

The function iterates backwards through the array while simultaneously removing elements from the end, causing misalignment between iteration and array state.

Impact

The severity of this vulnerability stems from fundamental data corruption in the position management system. When processing removals, the code's array manipulation logic creates fatal inconsistencies in position tracking data.

The corruption occurs through a precise technical sequence: when processing any given index i, the function first deletes data at that index and then pops from the end of the array. For example, when processing index 3 in an array [0,1,2,3,4], the delete operation creates [0,1,2,empty,4], followed by a pop resulting in [0,1,2,empty]. This operation irreversibly loses the data at index 4 and misaligns all subsequent array processing.

This implementation flaw cascades through the protocol's core functionality. Position tracking data becomes corrupted or lost, leading to incorrect fee calculations based on invalid position values. The FILO ordering - critical for the protocol's uplift fee assessment - breaks down completely. The system may double-count positions in fee calculations or miss them entirely, while the remaining position data in the protocol state becomes unreliable. These issues compound with each removal operation, progressively degrading the protocol's state.

The cumulative effect fundamentally undermines the protocol's ability to maintain accurate position management and fee assessments, potentially leading to permanent data loss and incorrect economic calculations across the system.

Recommended Mitigation Steps

  1. Revise the removal logic to maintain array integrity:

for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
localData.feeAmount += (depositAmount * feePerLP);
localData.amountLeft -= feeDataArray[i].amount;
lpNFT.burn(feeDataArray[i].tokenID);
// Shift remaining elements instead of using delete/pop
if (i != feeDataArray.length - 1) {
for (uint256 j = i; j < feeDataArray.length - 1; j++) {
feeDataArray[j] = feeDataArray[j + 1];
}
}
feeDataArray.pop();
if (localData.amountLeft == 0) break;
}
}

  1. Alternative approach using a separate array:

uint256[] memory indicesToRemove = new uint256[]();
uint256 removeCount = 0;
​
// Mark positions for removal
for (uint256 i = feeDataArray.length - 1; i >= 0; --i) {
if (/* removal condition */) {
indicesToRemove[removeCount++] = i;
}
}
​
// Remove marked positions from highest to lowest index
for (uint256 i = 0; i < removeCount; i++) {
uint256 index = indicesToRemove[i];
// Shift and pop for each marked position
}

The second approach may be more gas intensive but provides clearer and safer array management.

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.