Light ModeLight
Light ModeDark

One Bug Per Day

One H/M every day from top Wardens

Checkmark

Join over 455 wardens!

Checkmark

Receive the email at any hour!

Ad

Wrong invocation of Whirpools's updateFeesAndRewards will cause it to always revert

criticalCode4rena

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/liquidity_lockbox.sol#L277-L293

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.

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/liquidity_lockbox.sol#L277-L293

solidity
function 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.

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/interfaces/whirlpool.sol#L198

Whirlpool source code:

update_fees_and_rewards -> calculate_fee_and_reward_growths -> _calculate_modify_liquidity

https://github.com/orca-so/whirlpools/blob/3206c9cdfbf27c73c30cbcf5b6df2929cbf87618/programs/whirlpool/src/manager/liquidity_manager.rs#L97-L99

rs
fn _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