Liquid Staking

Summary

The bug allows totalQueuedShareWithdrawals to become inconsistent with the real queued amount if a deposit only partially covers the next withdrawal. This violates the totalQueuedSharesConsistency invariant.

Sequence of calls leading to the violation:

1. initialize is called to set up the contract state

2. queueWithdrawal is called to queue a withdrawal of 100 LST tokens

3. deposit is called to deposit 50 tokens, which partially finalizes the queued withdrawal

4. At this point, totalQueuedShares is inconsistent with the actual total queued withdrawals

Vulnerability Details

The problem occurs because totalQueuedShareWithdrawals is decremented by sharesToWithdraw before the loop that processes the queued withdrawals. If a deposit only partially covers the next queued withdrawal, totalQueuedShareWithdrawals will be decremented by the full sharesToWithdraw amount, even though only a portion of the withdrawal was processed. The _finalizeWithdrawals internal function in WithdrawalPool.sol: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/WithdrawalPool.sol#L422-L428

1. Assume the contract has the following state:

   • totalQueuedShareWithdrawals is 1000

   • There is a queued withdrawal of 800 shares

2. A deposit of 500 tokens is made, triggering _finalizeWithdrawals(500).

3. sharesToWithdraw is calculated as 500.

4. totalQueuedShareWithdrawals is decremented by 500, becoming 500.

5. The loop processes the queued withdrawal of 800 shares.

6. The withdrawal is partially finalized, with 500 shares processed and 300 shares remaining.

7. After the function execution, totalQueuedShareWithdrawals remains at 500, even though there are still 300 shares queued for withdrawal.

Impact

If totalQueuedShareWithdrawals is lower than the actual total of unprocessed withdrawals, it may incorrectly indicate that there are fewer queued withdrawals than there actually are.

Tools Used

Manual Review

Recommendations

By decrementing totalQueuedShareWithdrawals inline as withdrawals are processed, it ensures that totalQueuedShareWithdrawals always reflects the actual total of unprocessed queued withdrawals.