Liquid Staking

Summary

The withdraw function in the WithdrawalPool contract fails to properly update the totalQueuedShareWithdrawals and sharesRemaining values when processing partial withdrawals. this will potentially lead to issues(check impact section).

Vulnerability Details

In the withdraw function, when processing a partial withdrawal (i.e., when withdrawalId > batch.indexOfLastWithdrawal), the function only updates the partiallyWithdrawableAmount to zero. It does not adjust the totalQueuedShareWithdrawals or the sharesRemaining to reflect for the specific withdrawal.

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

Impact

-- Funds can become permanently locked in the contract. Since totalQueuedShareWithdrawals isn't decreased during partial withdrawals, it will remain artificially high. This means that even when all actual withdrawals have been processed, the contract will still think there are pending withdrawals preventing the release of the remaining funds.

-- Even more so, _finalizeWithdrawals function refeerences totalQueuedShareWithdrawals to process new withdrawals. An inflated totalQueuedShareWithdrawals will cause this function to process fewer withdrawals than it should, slowing down the entire withdrawal.

-- Also, checkUpkeep uses _getStakeByShares(totalQueuedShareWithdrawals) to determine if withdrawals should be executed. An inflated totalQueuedShareWithdrawals will cause this check to return true even when no actual withdrawals are pending,

Tools Used

Manual 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