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

_shouldQueueWithdrawal in PriorityPool._withdraw should refund the remaining tokens instead of reverting if set to false

peanuts

Summary

When withdrawing LST for asset tokens, _withdraw() uses _shouldQueueWithdrawal to check whether a withdrawal should be queued if the the full amount cannot be satisfied. If _shouldQueueWithdrawal is set to false, the function reverts instead of refunding the LST back to the user since the withdrawal cannot be fulfilled.

Vulnerability Details

In _withdraw(), the function first checks that there is any deposit tokens queued and withdraw asset tokens from there. If there are still tokens left to be withdrawn, the function will call queueWithdrawal() if _shouldQueueWithdrawal is set to true. Otherwise, the function reverts.

function _withdraw(
address _account,
uint256 _amount,
bool _shouldQueueWithdrawal
) internal returns (uint256) {
if (poolStatus == PoolStatus.CLOSED) revert WithdrawalsDisabled();
​
uint256 toWithdraw = _amount;
​
> if (totalQueued != 0) {
uint256 toWithdrawFromQueue = toWithdraw <= totalQueued ? toWithdraw : totalQueued;
​
totalQueued -= toWithdrawFromQueue;
depositsSinceLastUpdate += toWithdrawFromQueue;
sharesSinceLastUpdate += stakingPool.getSharesByStake(toWithdrawFromQueue);
toWithdraw -= toWithdrawFromQueue;
}
​
if (toWithdraw != 0) {
> if (!_shouldQueueWithdrawal) revert InsufficientLiquidity();
withdrawalPool.queueWithdrawal(_account, toWithdraw);
}
​
emit Withdraw(_account, _amount - toWithdraw);
return toWithdraw;
}

Note that the caller calling withdraw() has to deposit all the LST into the contract first.

For example, Alice calls withdraw and deposits 100 LST to the contract because she wants to withdraw 100 LST. There are 60 asset in queue. She will get 60 asset back first (assuming 1:1 conversion). For the remaining 40 LST, if she sets _shouldQueueWithdrawal to true, then she has to wait for the 40 assets. If she sets to false, the whole function reverts.

The function should refund the 40 LST back to Alice instead of reverting.

Impact

_shouldQueueWithdrawal always reverts if set to false.

Tools Used

Manual Review

Recommendations

Instead of reverting, return with the remaining toWithdraw amount and add another line in withdraw() to refund the remaining LST back to the caller.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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