Native ETH removeLiquidity reentrancy lets an LP swap against withdrawn stale reserves

Summary

Confirmed High in the Cantina public competition for revert-finance/stableswap-hooks (commit cf0c30e576f144809df9819f4d3ad49e0b7fe2d7).

removeLiquidity() sends native ETH to the withdrawing LP before updating reserves[] and before burning LP shares. For native ETH pools, poolManager.take() performs a full-gas external call to sender.receive(). Because that callback executes while the Uniswap v4 PoolManager is already unlocked, the receiver can call poolManager.swap() directly without calling poolManager.unlock() again.

The reentrant swap is priced by _beforeSwap() against the hook’s stale reserves[], which still include the liquidity already sent to the attacker. An attacker can supply temporary liquidity as phantom depth, withdraw it, and inside receive() buy almost the entire honest ETH reserve at the stale deep-pool price.

Affected code

In src/Liquidity.sol, _handleRemoveLiquidityCallback() performs external currency transfers inside a loop before updating internal reserves or burning shares:

• src/Liquidity.sol:207: burns the hook’s ERC-6909 claim on PoolManager for the withdrawn currency.
• src/Liquidity.sol:208: calls poolManager.take(currency, sender, amounts[i]). For native ETH (Currency.sol:45-48), this transfers ETH via a low-level .call{value: amount}("") to sender.
• src/Liquidity.sol:210: decrements reserves[i] -= amounts[i] only after poolManager.take() returns.
• src/Liquidity.sol:213: burns the LP shares only after the transfer loop completes.

During the outer unlockCallback, PoolManager is unlocked (PoolManager.sol:104-113), and PoolManager.swap() only checks onlyWhenUnlocked (PoolManager.sol:187-190). When sender.receive() calls poolManager.swap(), src/Swap.sol:100-106 prices the swap using the pre-withdrawal reserves[].

Proof of concept

Using RemoveLiquidityReentrancyPoC.t.sol against revert-finance/stableswap-hooks @ cf0c30e576f144809df9819f4d3ad49e0b7fe2d7:

forge test --match-path test/poc/RemoveLiquidityReentrancyPoC.t.sol -vv

Observed output:

[PASS] test_controlSameSwapAfterNormalRemovalCostsMuchMore()
Logs:
  control ETH taken 9999000000000000000000
  control token paid 100473376809521143097312

[PASS] test_removeLiquidityReentersSwapBeforeReservesAndSharesAreUpdated()
Logs:
  reentrant observed ETH reserve before swap 1000000000000000000000000
  reentrant ETH taken 9999000000000000000000
  reentrant token paid 10002990093097160574767
  final ETH reserve 1000000000000000000
  final token reserve 20002990093097160574767

• Control path (swap after withdrawal completes): Against the 10,000 ETH / 10,000 TOKEN honest pool, buying 9,999 ETH costs 100,473.38 TOKEN due to the StableSwap curve’s scarcity penalty near the bound.
• Exploit path (reentrant swap inside receive()): The hook still observes 1,000,000 ETH / 1,000,000 TOKEN reserves during the callback, so the attacker pays only 10,002.99 TOKEN for the same 9,999 ETH (over 10x cheaper), leaving honest LPs with 1 ETH and 20,002.99 TOKEN.

Impact

Any LP can flash-supply or temporarily deposit balanced liquidity into a native-ETH StableSwap pool, call removeLiquidity(), and re-enter poolManager.swap() during the ETH transfer callback to drain the honest ETH reserve without paying the curve’s depletion price.

Recommended fix

Enforce checks-effects-interactions in _handleRemoveLiquidityCallback(): decrement all reserves[i] and burn the LP shares before executing any poolManager.take() external transfers, and add a reentrancy lock across liquidity and swap hook callbacks.