Liquid Staking

Summary

In PriorityPool contract, the withdraw and unqueueTokens functions have a logical vulnerability which will allow an attacker to extract more tokens than he originally had queued.

Vulnerability Details

1. Initial State:

   • accountQueuedTokens[attacker] = 1000 (the attacker has 1000 queued tokens).

   • totalQueued = 2000 (there are a total of 2000 tokens currently queued across all accounts).

2. Attacker Calls withdraw:

   • The attacker initiates a withdrawal by calling the withdraw function with _shouldUnqueue = false.

   • The function checks _shouldUnqueue and, since it is false, it does not unqueue any tokens. Consequently, toWithdraw remains equal to _amountToWithdraw, which could be a high amount (let’s assume it is intentionally set to 1000 or significantly more).

3. Calling _withdraw:

   • Inside the withdraw function, since toWithdraw is greater than 0, it calls the internal _withdraw function.

   • In the _withdraw function, it checks totalQueued, which is 2000, so it proceeds to set toWithdrawFromQueue to the minimum of toWithdraw (which we have as 1000) and totalQueued (2000).

   • The contract deducts toWithdrawFromQueue from totalQueued, reducing it to 1000: totalQueued = totalQueued - toWithdrawFromQueue = 2000 - 1000 = 1000.

   • Note that accountQueuedTokens[attacker] remains at 1000, as no tokens have been unqueued in the first step.

4. End of Withdraw Call:

   • The _withdraw function then returns the amount of tokens that were not withdrawable because the contract only deducts from the total queue but does not affect the attacker’s queue.

   • The _withdraw function calls token.safeTransfer to transfer 1000 tokens to the attacker.

   • Now, the attacker gets 1000 token and is left with accountQueuedTokens[attacker] = 1000 and totalQueued = 1000.

5. Attacker Calls unqueueTokens:

   • Next, the attacker invokes the unqueueTokens function.

   • When calling unqueueTokens, the attacker can unqueue an amount of tokens (in this case, they can choose to unqueue their entire 1000).

   • Since there are 1000 queued tokens present (accountQueuedTokens[attacker]), the attacker can successfully unqueue these tokens.

6. Exploit Success:

   • The attacker successfully unqueues another 1000 tokens into their account.

   • As a result, the attacker now has effectively withdrawn 1000 tokens from the queue in addition to what was earlier processed by _withdraw, resulting in a total of 2000 tokens withdrawn from the system in an exploitative manner.

Impact

The attacker can manipulate _shouldUnqueue and totalQueued to extract more tokens than he originally had queued

Tools Used

Manual Review

Recommendations

To fix the logical vulnerability identified in the withdraw and unqueueTokens functions, we need to ensure that the mechanics of withdrawing and unqueuing tokens do not allow an attacker to exploit the contract and withdraw more tokens than they should be allowed. Here are a few recommendations:

1. Strict Validations Before Withdrawal: Before allowing a withdrawal and unqueueing process, the contract should ensure that the user has sufficient queued tokens to satisfy the total withdrawal request. Specifically, if a user is attempting to withdraw tokens, the system should validate not only the total queued amount but also the individual account queued tokens before allowing the operation.

2. Update Total Queued Balance After Unqueuing: After an unqueueing operation, ensure that the totalQueued and accountQueuedTokens are accurately updated and then re-check the available balance in withdraw to ensure that no additional tokens can be accessed post-withdrawal.

3. Prohibit Withdrawals Without Unqueueing First: If _shouldUnqueue is set to false, the withdrawal should not be permitted unless the user has existing tokens in accountQueuedTokens. Alternatively, consider disallowing withdrawal requests that might lead to inconsistencies by verifying against both account queued tokens and total queued tokens.