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

`minDeposits` missing check in `VaultCDepositController::withdraw` leaves the deposits in the vault under the minimum.

kiteweb3

Summary

The VaultCDepositController::withdraw contains the logic to withdraw tokens from the vault and send them to the staking pool. It handles the logic flaw for several scenarios:

  1. The withdrawal amount (toWithdraw) is greater than the vault's current deposits (deposits).

  2. The withdrawal amount is less than the vault's deposits and the withdrawal would leave less than the minimum deposits in the vault.

  3. Normal withdrawal (default case).

In the first scenario, the contract fails to check if the minDeposits is enough after the withdrawal. Leaving the vault under the minimum.

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

Vulnerability Details

function withdraw(uint256 _amount, bytes calldata _data) external {
if (!fundFlowController.claimPeriodActive() || _amount > totalUnbonded)
revert InsufficientTokensUnbonded();
​
GlobalVaultState memory globalState = globalVaultState;
uint64[] memory vaultIds = abi.decode(_data, (uint64[]));
VaultGroup memory group = vaultGroups[globalState.curUnbondedVaultGroup];
​
// withdrawals must continue with the vault they left off at during the previous call
if (vaultIds[0] != group.withdrawalIndex) revert InvalidVaultIds();
​
uint256 toWithdraw = _amount;
uint256 unbondedRemaining = totalUnbonded;
(uint256 minDeposits, ) = getVaultDepositLimits();
​
for (uint256 i = 0; i < vaultIds.length; ++i) {
// vault must be a member of the current unbonded group
if (vaultIds[i] % globalState.numVaultGroups != globalState.curUnbondedVaultGroup)
revert InvalidVaultIds();
​
group.withdrawalIndex = uint64(vaultIds[i]);
IVault vault = vaults[vaultIds[i]];
uint256 deposits = vault.getPrincipalDeposits();
​
if (deposits != 0 && vault.claimPeriodActive() && !vault.isRemoved()) {
if (toWithdraw > deposits) {
@> vault.withdraw(deposits);
unbondedRemaining -= deposits;
toWithdraw -= deposits;
} else if (deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits) {
// cannot leave a vault with less than minimum deposits
vault.withdraw(deposits);
unbondedRemaining -= deposits;
break;
} else {
vault.withdraw(toWithdraw);
unbondedRemaining -= toWithdraw;
break;
}
}
}
​
uint256 totalWithdrawn = totalUnbonded - unbondedRemaining;
​
token.safeTransfer(msg.sender, totalWithdrawn);
​
totalDeposits -= totalWithdrawn;
totalPrincipalDeposits -= totalWithdrawn;
totalUnbonded = unbondedRemaining;
​
group.totalDepositRoom += uint128(totalWithdrawn);
vaultGroups[globalVaultState.curUnbondedVaultGroup] = group;
}

Impact

Scenario:
toWithdraw = 50
deposits = 30
minDeposits = 20

The system enters the first if statement: toWithdraw = 50 > deposits = 30 returns true.

The system proceeds with the withdrawal of the all amount of the deposit (vault.withdraw(deposits)) leaving the vault empty and under the requested minimum deposit.

The minimum amount of deposits that a vault can hold is determined by the Chainlink staking contract. Vaults cannot have less than this minimum balance.

Tools Used

Manual review

Recommendations

Modify the first case in the withdrawal logic as follows:

if (toWithdraw > deposits) {
- vault.withdraw(deposits);
- unbondedRemaining -= deposits;
- toWithdraw -= deposits;
+ uint256 amountToWothdraw = deposits - minDeposits;
+ unbondedRemaining -= amountToWothdraw;
+ toWithdraw -= amountToWothdraw;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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