Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

As long as totalQueued > 0, the deposit logic will skip withdrawal process

Summary

The PriorityPool contract does not implement a mechanism to prioritize processing queued withdrawals over queued deposits. This oversight may result in unnecessary delays for users attempting to withdraw their funds, even when new deposits are available to fulfill those requests.

The function inline comment suggests that withdrawals should be prioritized before deposits or queuing, but in practice, the code does not ensure that withdrawals are always handled first if there are any queued deposits.

Vulnerability Details

Current Behavior:

  • The contract currently processes deposits based on the order they are received.

  • If there are existing queued deposits (totalQueued != 0), the contract does not attempt to process any queued withdrawals, regardless of how many withdrawal requests are pending.

The code you provided doesn’t fully reflect this intended behavior because of the issue with skipping withdrawal processing when totalQueued != 0. Specifically:

Queued withdrawals are not always prioritized as the logic to handle them is skipped when totalQueued > 0.
If there are any queued deposits, withdrawal requests are ignored, even though deposits are supposed to be used to handle them first.

if (totalQueued == 0) {
uint256 queuedWithdrawals = withdrawalPool.getTotalQueuedWithdrawals();
if (queuedWithdrawals != 0) {
uint256 toDepositIntoQueue = toDeposit <= queuedWithdrawals
? toDeposit
: queuedWithdrawals;
withdrawalPool.deposit(toDepositIntoQueue);
toDeposit -= toDepositIntoQueue;
}
}

This block is structured in a way that only handles withdrawals if totalQueued == 0. If there are queued deposits (i.e., totalQueued != 0), it skips processing withdrawals altogether. This can lead to a situation where withdrawal requests pile up while new deposits are still being processed or queued, creating liquidity issues.
Initial State:

  • Queued Withdrawals: 80 tokens

  • Queued Deposits: 50 tokens

  • User Action: Alice deposits 100 tokens into the contract.
    Contract Behavior:

    • Since totalQueued is not zero, the withdrawal processing logic is entirely skipped.

    • Alice’s deposit may either go into the staking pool or be added to queued deposits.
      Outcome:

      • The 80 tokens of queued withdrawal requests remain unfulfilled, despite Alice's deposit potentially being sufficient to cover them.

Expected Behavior:

  • The contract should prioritize processing queued withdrawals whenever new liquidity (deposits) is available, regardless of the presence of queued deposits.

  • If there are pending withdrawals, they should be fulfilled first before processing new deposits to ensure users can access their funds promptly.

Impact

  1. Users requesting withdrawals may face unnecessary delays.

  2. Liquidity Issue:

  • Assume there are 80 tokens in queued withdrawals and 50 tokens in queued deposits.

  • Alice deposits 100 tokens into the system.

  • If the contract doesn't prioritize withdrawals, Alice's deposit might go toward fulfilling the queued deposits instead of the withdrawal requests.

  • This leads to a scenario where the 80 tokens of withdrawal requests remain unfulfilled, even though Alice’s deposit could cover them.

  • Users who request withdrawals have to wait unnecessarily even though enough liquidity (Alice’s deposit) exists to pay them out.

Tools Used

Manual review

Recommendations

The function should always attempt to satisfy queued withdrawals first, regardless of whether there are queued deposits, the

uint256 queuedWithdrawals = withdrawalPool.getTotalQueuedWithdrawals();
if (queuedWithdrawals != 0) {
uint256 toDepositIntoQueue = toDeposit <= queuedWithdrawals ? toDeposit : queuedWithdrawals;
withdrawalPool.deposit(toDepositIntoQueue);
toDeposit -= toDepositIntoQueue;
}

After withdrawals are processed, the function should handle the remaining deposit by either sending it to the staking pool or queuing it.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.