Liquid Staking

Summary

In the _finalizeWithdrawals() function of the WithdrawalPool contract, the sharesRemaining field of fully processed withdrawals is not updated to reflect that the withdrawal has been finalized. This omission leads to significant accounting issues, causing finalized withdrawals to appear as still active in subsequent withdrawal logic. The error impacts both the integrity of the withdrawal data and the correct calculation of withdrawable funds, leading to the risk of over-withdrawal, incorrect batch processing, and potential user-facing errors.

Vulnerability Details

1. Failure to Update sharesRemaining

The vulnerability occurs in the following section of the _finalizeWithdrawals() function:

The queuedWithdrawals[i].sharesRemaining field is never updated to 0 even though we withdrawed these remaning shares. As a result, the contract continues to treat the withdrawal as if it still has shares remaining, even though all shares have already been withdrawn.

Impact of Not Updating sharesRemaining:

1. Incorrect Accounting of Withdrawals:
   The function getWithdrawalIdsByOwner() will misinterpret finalized withdrawals as still partially active because it relies on the sharesRemaining value to determine whether a withdrawal has been completed. This leads to incorrect calculations in the getFinalizedWithdrawalIdsByOwner() as it depends on getWithdrawalIdsByOwner() function, potentially causing users to withdraw more than they are entitled to.

2. Double Withdrawal Risk:
   If the system believes that there are remaining shares, it may allow the user to attempt another withdrawal, risking a scenario where withdrawals are processed twice or an incomplete set of withdrawals remains pending.

3. Batching Issues:
   Finalized withdrawals will still be considered as having outstanding shares, which can affect batching logic and prevent the correct processing of future withdrawals.

2. Failure to Update Withdrawal Info on Full Finalization

In another section of the same function, there is a similar omission when finalizing a withdrawal:

In this case, even though the contract progresses to the next withdrawal and updates the batch, the information of the fully finalized withdrawal is not updated. The sharesRemaining and possibly partiallyWithdrawableAmount fields are not cleared, leaving finalized withdrawals in an inconsistent state.

Impact of Not Updating Withdrawal Info:

1. Inconsistent State:
   Fully finalized withdrawals will remain in the system as active, which could lead to multiple issues, including incorrect data in future queries and further processing of finalized withdrawals. This impacts user-facing functions such as querying finalized withdrawals (getFinalizedWithdrawalIdsByOwner()), leading to incorrect results.

2. Over-execution of Withdrawals:
   Users may unintentionally withdraw more than they should, or the system may attempt to finalize a withdrawal multiple times, increasing the risk of gas inefficiency and errors in state management.

Impact

1. Over-withdrawal Risk:
   Users may be able to withdraw more than they are entitled to because the contract continues to treat finalized withdrawals as partially active. This poses a risk to the integrity of the fund pool.

2. Double Withdrawal Attempts:
   Users might be able to submit multiple withdrawal attempts on the same finalized withdrawals, leading to unintended contract behavior, user confusion, or financial loss.

3. Incorrect Finalized Withdrawal Queries:
   The function getFinalizedWithdrawalIdsByOwner() will provide incorrect results, causing users to see pending withdrawals as still active. This will lead to poor user experience and may introduce security risks related to incorrect data handling.

4. Batch Processing Issues:
   Finalized withdrawals might still be included in batch operations, leading to incorrect batching and potential failure of future withdrawal operations.

Tools Used

Manual code review

Recommendations

Update sharesRemaining: Ensure that the sharesRemaining field is updated to 0 when a withdrawal is fully finalized. This will prevent the contract from mistakenly treating finalized withdrawals as active in future logic.