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

_finalzeWithdrawals overwrites the token per share

Dup1337

Summary

In the WithdrawalPool contract, queued withdrawals are processed using the current share-to-stake ratio at the time of finalization rather than the ratio at the time the withdrawal was requested.

This discrepancy can lead to users receiving less (or more) tokens than expected due to changes in the share ratio over time.
It arises because the partiallyWithdrawableAmount is calculated using the current share ratio, which may have changed since the withdrawal request was queued.

Vulnerability Details

When a user requests a withdrawal, their withdrawal is queued with a certain amount of shares based on the share-to-stake ratio at that time. However, the share ratio can change over time due to staking rewards, slashing, or other factors affecting the total stake and total shares.

In the _finalizeWithdrawals function, when partially finalizing a withdrawal, the contract updates the partiallyWithdrawableAmount by converting the sharesToWithdraw to stake using the current share-to-stake ratio:

Contract: WithdrawalPool.sol
​
422: function _finalizeWithdrawals(uint256 _amount) internal {
423: uint256 sharesToWithdraw = _getSharesByStake(_amount);
424: uint256 numWithdrawals = queuedWithdrawals.length;
425:
426: totalQueuedShareWithdrawals -= sharesToWithdraw;
427:
428: for (uint256 i = indexOfNextWithdrawal; i < numWithdrawals; ++i) {
429: uint256 sharesRemaining = queuedWithdrawals[i].sharesRemaining;
430:
431: if (sharesRemaining < sharesToWithdraw) {
432: // fully finalize withdrawal
433: sharesToWithdraw -= sharesRemaining;
434: continue;
435: }
436:
437: if (sharesRemaining > sharesToWithdraw) {
438: // partially finalize withdrawal
439: queuedWithdrawals[i] = Withdrawal(
440: uint128(sharesRemaining - sharesToWithdraw),
441: uint128(
442: queuedWithdrawals[i].partiallyWithdrawableAmount +
443: >>> _getStakeByShares(sharesToWithdraw)
444: )
445: );
446: indexOfNextWithdrawal = i;
447: withdrawalBatches.push(
448: WithdrawalBatch(uint128(i - 1), uint128(_getStakeByShares(1 ether)))
449: );
450: } else {
451: // fully finalize withdrawal
452: indexOfNextWithdrawal = i + 1;
453: withdrawalBatches.push(
454: WithdrawalBatch(uint128(i), uint128(_getStakeByShares(1 ether)))
455: );
456: }
457:
458: sharesToWithdraw = 0;
459: break;
460: }
461:
462: // entire amount must be accounted for
463: assert(sharesToWithdraw == 0);
464:
465: emit WithdrawalsFinalized(_amount);
466: }

This means that the amount of tokens the user can withdraw is calculated using the share ratio at the time of finalization, not the ratio at the time the withdrawal was requested. If the share ratio has decreased (e.g., due to slashing or a drop in staking rewards), the user will receive less stake than they would have based on the original ratio, leading to potential financial loss.

The user´s fund is queued for withdrawal in queueWithdrawal triggered from priorityPool's withdraw function when the funds are nto sufficient to transfer to the user;

Contract: WithdrawalPool.sol
​
302: function queueWithdrawal(address _account, uint256 _amount) external onlyPriorityPool {
303: if (_amount < minWithdrawalAmount) revert AmountTooSmall();
304:
305: lst.safeTransferFrom(msg.sender, address(this), _amount);
306:
307: uint256 sharesAmount = _getSharesByStake(_amount);
308: queuedWithdrawals.push(Withdrawal(uint128(sharesAmount), 0));
309: totalQueuedShareWithdrawals += sharesAmount;
310:
311: uint256 withdrawalId = queuedWithdrawals.length - 1;
312: queuedWithdrawalsByAccount[_account].push(withdrawalId);
313: withdrawalOwners[withdrawalId] = _account;
314:
315: emit QueueWithdrawal(_account, _amount);
316: }

  • The contract calculates the number of shares corresponding to _amount at the current share ratio.

  • A Withdrawal struct is created with shares and partiallyWithdrawableAmount set to 0.

  • The withdrawal is added to queuedWithdrawals.

And once _finalizeWithdrawals is triggered (either by a deposit from the priorityPool or upKeep action ), below flow is executed:

  • The function processes each queued withdrawal, converting shares back to stake using the current share ratio.

  • If only part of the withdrawal can be finalized, it updates the partiallyWithdrawableAmount accordingly (L:443).

And the problematic part is this:

  • The shares were calculated based on the share ratio at the time of the withdrawal request.

  • The conversion back to stake uses the current share ratio, which may have changed especially if there are rewards in the staking pools (or a slashing) but no deposit action took place in between.

This ends with:

  • If the share ratio decreases, users receive less stake for their shares.

  • Users experience a loss compared to the expected amount at the time of their request.

Impact

Financial Loss to Users

Tools Used

Manual Code Review

Recommendations

  • Modify the Withdrawal struct to include the share-to-stake ratio at the time of the request

struct Withdrawal {
uint128 sharesRemaining;
uint128 partiallyWithdrawableAmount;
+ uint256 shareRatioAtRequest;
}

and use stored share ratio for conversions.

Updates

Lead Judging Commences

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

Support

FAQs

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