Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: high
Invalid

Wrong argument passed to `vault.withdraw` in the `VaultDepositController::withdraw` function which will break a protocol invariant.

Relevant GitHub Links

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L140

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L141

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L142

Summary

The cannot leave a vault with less than minimum deposits protocol requirement will be bypassed in the VaultDepositController::withdraw function due to a miscalculation of the amount to withdraw.

Vulnerability Details

When withdraw to the vault with the VaultDepositController::withdraw function, when the condition deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits is true we must withdraw the funds, ensuring that at least the minimum deposits is left in the vault. But with the current implementation, when this condition is true, everything is withdrawn from the vault and the vault will be left with less than the minimum deposits:

function withdraw(uint256 _amount, bytes calldata _data) external {
/// ... The rest of code
for (uint256 i = 0; i < vaultIds.length; ++i) {
/// ... The rest of code
if (deposits != 0 && vault.claimPeriodActive() && !vault.isRemoved()) {
if (toWithdraw > deposits) {
/// ... The rest of code
@> } else if (deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits) {
@> // cannot leave a vault with less than minimum deposits
@> vault.withdraw(deposits);
unbondedRemaining -= deposits;
break;
} else {
/// ... The rest of code
}
}
}
/// ... The rest of code
}

Impact

- Denial of Service (DoS): Allowing withdrawals that leave a vault with insufficient funds might disrupt operations; future withdrawals from the vault may be blocked if the address's balance is too low to meet withdrawal requests.

- Potential Exploits by Malicious Actors: A faulty withdrawal system can be a vulnerability, allowing an attacker to manipulate the system to drain liquidity or take advantage of misallocated rewards.

Tools Used

Manual review.

Recommendations

function withdraw(uint256 _amount, bytes calldata _data) external {
/// ... The rest of code
for (uint256 i = 0; i < vaultIds.length; ++i) {
/// ... The rest of code
if (deposits != 0 && vault.claimPeriodActive() && !vault.isRemoved()) {
if (toWithdraw > deposits) {
/// ... The rest of code
} else if (deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits) {
// cannot leave a vault with less than minimum deposits
- vault.withdraw(deposits);
+ // @audit Calculate the amount that can be withdrawn to leave exactly the minimum deposit
+ uint256 withdrawableAmount = deposits - minDeposits;
+ // @audit Withdraw only the amount that leaves the vault with the minimum deposit
+ vault.withdraw(withdrawableAmount);
unbondedRemaining -= deposits;
+ toWithdraw -= withdrawableAmount;
break;
} else {
/// ... The rest of code
}
}
}
/// ... The rest of code
}
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.