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

user loses tokens in partial withdrawals

kwakudr

Summary

user loses tokens in partial withdrawals.

Vulnerability Details

function withdraw(uint256[] calldata _withdrawalIds, uint256[] calldata _batchIds) external {
address owner = msg.sender;
uint256 amountToWithdraw;

for (uint256 i = 0; i < _withdrawalIds.length; ++i) {
uint256 withdrawalId = _withdrawalIds[i];
Withdrawal memory withdrawal = queuedWithdrawals[_withdrawalIds[i]];
uint256 batchId = _batchIds[i];
WithdrawalBatch memory batch = withdrawalBatches[batchId];
​
if (withdrawalOwners[withdrawalId] != owner) revert SenderNotAuthorized();
if (
batchId != 0 && withdrawalId <= withdrawalBatches[batchId - 1].indexOfLastWithdrawal
) revert InvalidWithdrawalId();
if (
batchId != 0 &&
withdrawalId > batch.indexOfLastWithdrawal &&
withdrawal.partiallyWithdrawableAmount == 0
) revert InvalidWithdrawalId();
​
if (withdrawalId <= batch.indexOfLastWithdrawal) {
amountToWithdraw +=
withdrawal.partiallyWithdrawableAmount +
(uint256(batch.stakePerShares) * uint256(withdrawal.sharesRemaining)) /
1e18;
delete queuedWithdrawals[withdrawalId];
delete withdrawalOwners[withdrawalId];
} else {
amountToWithdraw += withdrawal.partiallyWithdrawableAmount;
queuedWithdrawals[withdrawalId].partiallyWithdrawableAmount = 0;
}
}
​
token.safeTransfer(owner, amountToWithdraw);
emit Withdraw(owner, amountToWithdraw);
}

The issue of incomplete withdrawal processing occurs in the section I've highlighted above. Let's break it down:

  1. The function iterates through the queuedWithdrawals array.

  2. When it encounters a withdrawal where sharesRemaining < sharesToWithdraw, it does the following:

    • Reduces sharesToWithdraw by sharesRemaining

    • Continues to the next iteration of the loop without processing the current withdrawal

This approach is problematic for several reasons:

  1. The current withdrawal is not processed at all. The function doesn't update the withdrawal record or transfer any tokens to the user.

  2. The sharesRemaining for this withdrawal are effectively "lost" in the system. They're subtracted from sharesToWithdraw, but not accounted for anywhere else.

  3. The user who requested this withdrawal will never receive their funds through this mechanism.

Impact

User loses their tokens

Tools Used

Manual review

Recommendations

We process every withdrawal we encounter, whether it's smaller than, equal to, or larger than sharesToWithdraw.

  • For withdrawals smaller than or equal to sharesToWithdraw, we process them fully.

  • For withdrawals larger than sharesToWithdraw, we process them partially.

  • We keep track of sharesToWithdraw and stop when we've processed all requested shares.

Updates

Lead Judging Commences

inallhonesty Lead Judge
11 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.