Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Incorrect Token Transfer in executeQueuedWithdrawals Function Leads to Locked Funds

allengeorge

Summary:

The executeQueuedWithdrawals function in the PriorityPool contract contains a critical flaw in its token flow, potentially leading to stuck funds and failed withdrawals.

Vulnerability Details:

In the executeQueuedWithdrawals function:

function executeQueuedWithdrawals(uint256 _amount, bytes[] calldata _data) external onlyWithdrawalPool {
IERC20Upgradeable(address(stakingPool)).safeTransferFrom(msg.sender, address(this), _amount);
stakingPool.withdraw(address(this), address(this), _amount, _data);
token.safeTransfer(msg.sender, _amount);
}

The issue lies in the stakingPool.withdraw call. It uses address(this) (PriorityPool) as both the account to withdraw from and the receiver of the withdrawn tokens. This is incorrect because:

  1. The PriorityPool doesn't own the LST tokens it's trying to burn.

  2. The underlying tokens are being sent to the PriorityPool instead of directly to the WithdrawalPool.

Proof of Concept:

  1. WithdrawalPool calls executeQueuedWithdrawals on PriorityPool with _amount = 100 tokens.

  2. LST tokens are transferred from WithdrawalPool to PriorityPool.

  3. PriorityPool calls stakingPool.withdraw(address(this), address(this), 100, _data).

  4. StakingPool attempts to burn 100 LST from PriorityPool's balance, which fails because PriorityPool doesn't have these tokens (they were just transferred and not approved).

  5. The withdrawal fails, and the LST tokens are now stuck in the PriorityPool.

Here's how the StakingPool's withdraw function works:

function withdraw(address _account, address _receiver, uint256 _amount, bytes[] calldata _data)
external
onlyPriorityPool
{
uint256 toWithdraw = _amount;
if (_amount == type(uint256).max) {
toWithdraw = balanceOf(_account);
}
​
uint256 balance = token.balanceOf(address(this));
if (toWithdraw > balance) {
_withdrawLiquidity(toWithdraw - balance, _data);
}
require(token.balanceOf(address(this)) >= toWithdraw, "Not enough liquidity available to withdraw");
​
_burn(_account, toWithdraw);
totalStaked -= toWithdraw;
token.safeTransfer(_receiver, toWithdraw);
}

The _burn(_account, toWithdraw) call will fail because the PriorityPool (specified as _account) doesn't have the LST balance and approval to burn.

Impact:

  • Users' withdrawal requests will fail, leaving their funds locked in the protocol.

  • LST tokens will be stuck in the PriorityPool, leading to a discrepancy between the protocol's accounting and actual token distribution.

  • This could lead to a loss of funds for users and damage the protocol's functionality and reputation.

Tools Used:

Manual review

Recommendations:

Modify the executeQueuedWithdrawals function to correctly specify the WithdrawalPool as the account to withdraw from and receive tokens:

function executeQueuedWithdrawals(uint256 _amount, bytes[] calldata _data) external onlyWithdrawalPool {
IERC20Upgradeable(address(stakingPool)).safeTransferFrom(msg.sender, address(this), _amount);
stakingPool.withdraw(msg.sender, msg.sender, _amount, _data);
// Remove this line as it's no longer needed:
// token.safeTransfer(msg.sender, _amount);
}

This change ensures that:

  1. LST tokens are burned directly from the WithdrawalPool's balance.

  2. Underlying tokens are sent directly to the WithdrawalPool.

  3. There's no unnecessary token transfer through the PriorityPool.

These modifications will prevent the funds from getting stuck and ensure the withdrawal process works as intended.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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