Liquid Staking

Summary

The withdraw function in the WithdrawalPool contract fails to properly update the totalQueuedShareWithdrawals and sharesRemaining values when processing partial withdrawals. This can lead to funds becoming locked in the contract, inefficient processing of withdrawals, and incorrect upkeep checks.

Vulnerability Details

In the withdraw function, when processing a partial withdrawal, the function only updates the partiallyWithdrawableAmount to zero without adjusting the totalQueuedShareWithdrawals or the sharesRemaining for the specific withdrawal.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/WithdrawalPool.sol#L287-L290

This code snippet is located at lines 288-290 in the WithdrawalPool.sol file.

Impact

1. Funds can become permanently locked in the contract due to an artificially high totalQueuedShareWithdrawals.

2. The _finalizeWithdrawals function may process fewer withdrawals than it should, slowing down the entire withdrawal process.

3. The checkUpkeep function may return true even when no actual withdrawals are pending, leading to unnecessary upkeep operations.

4. The contract's state does not accurately reflect the actual pending withdrawals after partial withdrawals are processed, potentially leading to inconsistencies in withdrawal management.

Tools Used

Manual code review

Recommendations

Update the withdraw function to properly adjust both totalQueuedShareWithdrawals and sharesRemaining when processing partial withdrawals.
Calculate the number of shares corresponding to the partiallyWithdrawableAmount, then subtract this value from both totalQueuedShareWithdrawals and the specific withdrawal's sharesRemaining.
Consider adding a new variable to track the total partially withdrawable amount across all withdrawals, which can be used to accurately calculate the total pending withdrawals.
Implement thorough unit tests to ensure correct behavior of partial withdrawals and accurate state updates.
Consider adding a function to recalculate and correct the totalQueuedShareWithdrawals based on actual pending withdrawals, which can be used as a failsafe if discrepancies are detected.