Liquid Staking

Summary

The Protocol is designed in such a way that a user can withdraw their tokens from the Protocol or queue them for withdrawal when there is no available liquidity at the time of the withdrawal request by setting the _shouldQueueWithdrawal parameter as true in the PriorityPool::withdraw function. The Protocol is then supposed to fulfill the queued withdrawals anytime there is liquidity in the Protocol e.g. when a deposit is made by some other user.
Unfortunately, the Protocol's code is not designed to fulfull queued withdrawals whenever the Protocol receives some liquidity.

Vulnerability Details

The problem lies in the fact that there is no way the Protocol can fulfull queued withdrawals. In fact, the function that looks as though it was supposed to fulfill this role, has no way of sending queued withdrawal tokens to the users who queued for withdrawal. The WithdrawalPool::_finalizeQueuedWithdrawal function only finalizes accounting after withdrawals are finalized but does not send any tokens to a user. The function is given in the code snipet below:

Here is the github link to the above function https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/WithdrawalPool.sol#L422-L466

Two things to note about the above function:

1. When withdrawals are fully finalized, no tokens are sent to the user who queued withdrawals initially and the Withdrawal struct containing the queued withdrawal information is not updated

2. When withdrawals are partially finalized, no tokens are sent to the user who queued withdrawals initially but the Withdrawal struct containing the queued withdrawal information is updated

Either ways, no tokens are sent to the user.

Impact

The impact is that user funds that are queued for withdrawal are stuck in the Protocol and the user loses their funds.
To further explain this, there are two ways intuitively how the Protocol could fulfill queued withdrawals. One way is when another user deposits tokens into the Protocol and the second way is when the Protocol performs an upkeep. Now lets examine the two possible ways:

1. When a User deposits some tokens into the protocol i.e. user1 -> PriorityPool::deposit. The amount is transfered from the user to the Priority Pool; thereafter the Priority Pool calls the _deposit function i.e. PriorityPool::deposit -> PriorityPool::_deposit.
   The Priority Pool calls the deposit function in the Withdrawal Pool i.e. PriorityPool::_deposit -> WithdrawalPool::deposit. The required tokens are then transferred from the Priority Pool to the Withdrawal Pool while the same amount of lst (liquid staking tokens) are transfered from the Withdrawal Pool to the Priority Pool. Then the _finalizeWithrawals function is called i.e. WithdrawalPool::deposit -> WithdrawalPool::_finalizeWithrawals. At this point, the entire path sums up as
   user -> PriorityPool::deposit -> PriorityPool::_deposit -> WithdrawalPool::deposit -> WithdrawalPool::_finalizeWithrawals
   The _finalizeWithdrawals function finalizes withdrawal accounting after withdrawals have been executed. However, in the execution path above, no tokens were transfered to users who had their withdrawals queued and yet, withdrawal accounting was finalized. In summary, the Protocol now has liquidity but users who had their withdrawals queued still have their token stuck in the Protocol

2. When the WithdrawalPool performs an upkeep by calling the WithdrawalPool::performUpkeep function, the WithdrawalPool calls the PriorityPool::executeQueuedWithdrawals function i.e. WithdrawalPool::performUpkeep -> PriorityPool::executeQueuedWithdrawals which sends tokens from the Priority Pool to the Withdrawal Pool. Thereafter, the perform upkeep function calls the _finalizeWithdrawals function i.e. WithdrawalPool::performUpkeep -> WithdrawalPool::_finalizeWithdrawals. Again, no tokens were transfered to users who had their withdrawals queued and yet, withdrawal accounting was finalized. As a result, users who had their withdrawals queued still have their token stuck in the Protocol

Tools Used

Manual Review and Hardhat

Proof of Concept:

1. User A deposits 100 tokens into the protocol. User A now have 100 tokens less than their initial balance

2. User A attempts to withdraw 20 tokens out of the 100 tokens they deposited earlier into the protocol. User A sets the _shouldQueueWithdrawal option as true.

3. Now because the Protocol does not have liquidity, the withdrawal request of user A is queued

4. Another User B deposits 15 tokens into the Protocol. User A's withrawal request is still not fulfilled

5. Another User C deposits 5000 tokens into the Protocol. This deposit provides more liquidity to the Protocol but User A's withdrawal request is still not fulfilled. MEanwhile the Priority Pool holds 4095 tokens

6. User C withdraws 2000 tokens out of their initial deposit leaving 2095 tokens in the Priority Pool

7. User B also withdraws the 15 tokens they deposited earlier into the protocol.

8. User A attempts to withdraw their 100 tokens since the queued withdrawal was not fulfilled but the call reverts with "Transfer amount exceeds balance" message

9. User A then attempts to withdraw 80 tokens and the call succeeds. Now, User A has 20 tokens less than their initial balance.

Recommendations

The WithdrawalPool::_finalizeWithdrawals function should be modified to transfer the required amount to the user and update the Withdrawal struct as necessary