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.
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:
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.
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.
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.
User Action: Alice deposits liquidity into the pool using addLiquidityProportional(), which records her deposit in the FeeData array and mints an LPNFT.
Withdrawal Attempt: Alice attempts to withdraw her liquidity via removeLiquidityProportional().
Hook Execution: The onAfterRemoveLiquidity() function is invoked, initiating the faulty backward iteration over the FeeData array.
Loop Underflow: The loop starts with i = feeDataArrayLength - 1. As i decrements to 0 and then attempts to decrement further, it underflows.
Transaction Revert: Solidity detects the underflow and reverts the transaction, preventing Alice from withdrawing her funds.
Manual review
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.
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.