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 {
for (uint256 i = 0; i < vaultIds.length; ++i) {
if (deposits != 0 && vault.claimPeriodActive() && !vault.isRemoved()) {
if (toWithdraw > deposits) {
@> } else if (deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits) {
@>
@> vault.withdraw(deposits);
unbondedRemaining -= deposits;
break;
} else {
}
}
}
}
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
}