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

Incorrect withdrawal amount calculation in `VaultControllerStrategy`

bladesec

Summary

In withdraw function of VaultControllerStrategy contract, it calculates the withdrawal amount from vaults incorrectly, which results in failure in revert or withdraw full amount from a vault.

Vulnerability Details

When withdrawal happens in VaultControllerStrategy contract, it iterates through registered vaults and try to withdraw more than expected from a vault, as implemented follows:

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

The issue arises when the principal deposits amount of a vault exceeds the withdrawal amount and the remaining amount is less than minimum deposit amount of a strategy.

The minDeposits represents the minimum assets a vault must hold, but it tries to withdraw full deposits from the vault.

Impact

  • Revert of withdraw

  • Withdraw more than expected amount from a vault

Tools Used

Manual Review

Recommendations

Instead of withdrawing the whole deposits, it should withdraw deposits - minDeposits so that the vault satisfies the minimum amount deposited.

Can be fixed as follows:

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;
+ vault.withdraw(deposits - minDeposits);
+ unbondedRemaining -= deposits - minDeposits;
break;
} else {
vault.withdraw(toWithdraw);
unbondedRemaining -= toWithdraw;
break;
}
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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