Liquid Staking

Summary

The withdraw function in the VaultDepositController contract can withdraw and transfer more tokens than requested by the user under certain conditions. This can lead to unexpected token distributions and accounting inaccuracies.

Vulnerability Details

The issue can be found in the withdraw function of the VaultDepositController contract.
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L135-L140

When the remaining balance in a vault after the requested withdrawal would be less than minDeposits, the function withdraws the entire vault balance instead of the requested amount.

Impact

if you take make a scenario with the following initial conditions:

• Vault deposit: 1000 tokens

• Minimum deposit (minDeposits): 100 tokens

• User requests withdrawal: 800 tokens

Mathematical breakdown:

1. Requested withdrawal: 800 tokens

2. Remaining balance if request fulfilled: 1000 - 800 = 200 tokens

3. Condition check: 200 > 0 && 200 < 100 (minDeposits)

4. Actual withdrawal: 1000 tokens (entire vault balance)

issue to look at
Tokens over-withdrawn: 1000 - 800 = 200 tokens
Percentage of excess withdrawal: (200 / 800) * 100 = 25%

Impact on the contract:
totalUnbonded: Reduced by 1000 instead of 800 (25% more reduction)
totalDeposits: Reduced by 1000 instead of 800 (25% more reduction)
totalPrincipalDeposits: Reduced by 1000 instead of 800 (25% more reduction)

if this issue pile up it can compound over multiple withdrawals, potentially leading to significant accounting errors and unexpected token distributions.

Tools Used

Manual

Recommendations

1. Implement a cap on the withdrawal amount:

1. Update accounting based on actual withdrawn amount:

1. Add a check to ensure withdrawal doesn't exceed requested amount:

1. If the behavior is intentional, rename the function and add clear documentation:

1. Add events for transparency: