Wrong invocation of Whirpools's updateFeesAndRewards will cause it to always revert
criticalLines of code
Vulnerability details
Impact
Deposits will be unwithdrawable from the lockbox
Proof of Concept
If the entire liquidity of a position has been removed, the withdraw function calls the updateFeesAndRewards
function on the Orca pool before attempting to close the position.
solidityfunction withdraw(uint64 amount) external { address positionAddress = positionAccounts[firstAvailablePositionAccountIndex]; ...... uint64 positionLiquidity = mapPositionAccountLiquidity[positionAddress]; ...... uint64 remainder = positionLiquidity - amount; ...... if (remainder == 0) { // Update fees for the position AccountMeta[4] metasUpdateFees = [ AccountMeta({pubkey: pool, is_writable: true, is_signer: false}), AccountMeta({pubkey: positionAddress, is_writable: true, is_signer: false}), AccountMeta({pubkey: tx.accounts.tickArrayLower.key, is_writable: false, is_signer: false}), AccountMeta({pubkey: tx.accounts.tickArrayUpper.key, is_writable: false, is_signer: false}) ]; whirlpool.updateFeesAndRewards{accounts: metasUpdateFees, seeds: [[pdaProgramSeed, pdaBump]]}();
This is faulty as the updateFeesAndRewards
function will always revert if the position's liquidity is 0.
Whirlpool source code:
update_fees_and_rewards -> calculate_fee_and_reward_growths -> _calculate_modify_liquidity
rsfn _calculate_modify_liquidity( whirlpool: &Whirlpool, position: &Position, tick_lower: &Tick, tick_upper: &Tick, tick_lower_index: i32, tick_upper_index: i32, liquidity_delta: i128, timestamp: u64, ) -> Result<ModifyLiquidityUpdate> { // Disallow only updating position fee and reward growth when position has zero liquidity if liquidity_delta == 0 && position.liquidity == 0 { return Err(ErrorCode::LiquidityZero.into()); }
Since the withdrawal positions are chosen sequentially, only a maximum of (first position's liquidity - 1) amount of liquidity can be withdrawn.
POC Test
https://gist.github.com/10xhash/a687ef66de8210444a41360b86ed4bca
Tools Used
Manual review
Recommended Mitigation Steps
Avoid the update_fees_and_rewards
call completely since fees and rewards would be updated in the decreaseLiquidity
call.
Assessed type
call/delegatecall