QuantAMM

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

Withdrawal Function Reverts Due to Improper Loop Handling, Locking User Funds

Vulnerability Deatils

The UpliftOnlyExample smart contract contains a critical flaw in the onAfterRemoveLiquidity() function that causes withdrawal transactions to revert. This issue arises from an incorrect backward iteration over the FeeData array using an unsigned integer, preventing users from accessing their deposited funds.

Detailed Description

The UpliftOnlyExample contract manages liquidity provision and withdrawal in a Balancer-like pool, tracking user deposits and applying fees based on the uplift in pool value since each deposit. Users add liquidity through the addLiquidityProportional() function, which records each deposit in a FeeData array associated with their address and the pool. Each deposit is also tied to a unique NFT (LPNFT) representing the user's position.

When a user attempts to withdraw liquidity via the removeLiquidityProportional() function, the onAfterRemoveLiquidity() hook is triggered. This function is responsible for calculating and charging fees based on the uplift since each deposit. To process the fees, the function iterates backward through the user's FeeData array using the following loop:

for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {
// Loop body
}

However, this loop uses an unsigned integer i with the condition i >= 0. Since uint256 is always non-negative, the condition i >= 0 is perpetually true. When i decrements below 0, it underflows, causing i to wrap around to 2^256 - 1. Solidity 0.8.x enforces underflow checks, resulting in a transaction revert when the loop attempts to decrement i below 0.

This flawed loop logic prevents the onAfterRemoveLiquidity() function from completing successfully, thereby blocking users from withdrawing their liquidity.

Impact

High: Users are unable to withdraw their liquidity, resulting in a severe loss of access to funds and causing permanent disruption of the liquidity pool's withdrawal functionality.

Impact Explanation

The vulnerability directly affects all users attempting to withdraw liquidity, rendering their funds inaccessible. This undermines the core functionality of the protocol, leading to significant financial and trust-related repercussions.

Proof of Concept

  1. User Action: Alice deposits liquidity into the pool using addLiquidityProportional(), which records her deposit in the FeeData array and mints an LPNFT.

  2. Withdrawal Attempt: Alice attempts to withdraw her liquidity via removeLiquidityProportional().

  3. Hook Execution: The onAfterRemoveLiquidity() function is invoked, initiating the faulty backward iteration over the FeeData array.

  4. Loop Underflow: The loop starts with i = feeDataArrayLength - 1. As i decrements to 0 and then attempts to decrement further, it underflows.

  5. Transaction Revert: Solidity detects the underflow and reverts the transaction, preventing Alice from withdrawing her funds.

Tools Used

Manual review

Recommended Mitigation Steps

Revise the loop in onAfterRemoveLiquidity() to use a safe backward iteration pattern by initializing i to feeDataArrayLength and decrementing i within a condition that ensures it never underflows.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_onAfterRemoveLiquidity_loop_underflow

That’s definitely not the best way to handle that but there is indeed no impact. If someone tries to get more than their deposits, it must revert, and thanks to that "fancy mistake"(or genius code ?), it does.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.