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

Incorrect withdrawal Amount in `VaultCDepositController::withdraw` leads to Vault financial loss

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 second scenario, this flaw causes the contract to withdraw an incorrect (greater than the requested ) amount of tokens from the vault, leading to a loss of funds for the vault.

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

Vulnerability Details

In the second case of the withdrawal logic, the contract checks if the withdrawal amount is less than the vault's deposits and the withdrawal would leave less than the minimum deposits in the vault. Then proceeds to withdraw the funds but it withdraws the full deposits amount that is greater than the intended toWithdraw amount requested. This results in an excessive withdrawal that leaves the vault with fewer funds than expected.

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;
}
}
}

Impact

Scenario:
toWithdraw = 20
deposits = 60
minDeposits = 50

toWithdraw = 20 > deposits = 60 the system enters the first elseif statement:

deposits - toWithdraw = 60 - 20 = 40 > 0 && deposits - toWithdraw = 60 - 20 = 40 < minDeposits = 50 that returns true

The system proceeds with the withdrawal (vault.withdraw(deposits)). But it withdraws the wrong amount (deposits = 60) which is greater than the amount requested (that was toWithdraw = 20). The system withdraws 40 more than necessary.

The contract transfers to the staking pool more funds than intended with a funds loss for the Vault.

Tools Used

Manual review

Recommendations

Modify the second case in the withdrawal logic as follows:

else if (deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits) {
// Withdraw only the amount that leaves minDeposits in the vault
+ uint256 amountToWithdraw = deposits - minDeposits;
- vault.withdraw(deposits);
+ vault.withdraw(amountToWithdraw);
- unbondedRemaining -= deposits;
+ unbondedRemaining -= amountToWithdraw;
+ toWithdraw -= amountToWithdraw;
break;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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