`deposit()` will be permanently DOSed if the vault's total assets ever becomes 0

Github username: @milotruck Submission hash (on-chain): 0x811ea4ceb76e6ce6fb7e737e45c7488c327f4a4c16ccf0c9d5d5364b2e482d5a Severity: low

Description:

Bug Description

In VaultState.sol, the _convertToShares() function will revert if _totalAssets if 0 while there are still shares:

VaultState.sol#L225-L236

  function _convertToShares(
    uint256 assets,
    Math.Rounding rounding
  ) internal view returns (uint256 shares) {
    uint256 totalShares = _totalShares;
    // Will revert if assets > 0, totalShares > 0 and _totalAssets = 0.
    // That corresponds to a case where any asset would represent an infinite amount of shares.
    return
      (assets == 0 || totalShares == 0)
        ? assets
        : Math.mulDiv(assets, totalShares, _totalAssets, rounding);
  }

As seen below, _convertToShares() is used to calculate the amount of shares to mint to a user when calling _deposit():

VaultEnterExit.sol#L167-L168

    // calculate amount of shares to mint
    shares = _convertToShares(assets, Math.Rounding.Up);

As such, if the vault's total assets ever becomes 0 while its total shares is non-zero, _deposit() will always revert, permanently DOSing future deposits.

Such a scenario could occur if a vault has exactly 32 ETH of assets and only 1 validator, and the validator gets slashed with the maximum penalty. For example:

Alice creates a new vault and deposits 1e9 ETH as the initial security deposit.
Afterwards, she deposits another 32 ether - 1e9 ETH into the vault, making both total assets and total shares 32e18.
She calls registerValidator() to stake the 32 ETH and register a validator.
After some time, her validator is slashed with the maximum correlation penalty, causing her entire stake to be slashed.
As a result, her vault experiences a 32 ETH loss.
Now, total assets is 0, while total shares is still 32e18.
As such, whenever she tries to call deposit(), _convertToShares() will revert as the denominator is 0.

While it is unlikely that a validator's entire stake will be slashed, note that it is technically possible due to the correlation penalty, as stated here:

The maximum slash is the full effective balance of all slashed validators (i.e. if there are lots of validators being slashed they could lose their entire stake).

Impact

If a vault experiences a huge loss and its total assets goes down to 0, deposits will be permanently DOSed forever.

Recommended Mitigation

In _convertToShares(), consider returning assets if _totalAssets is zero:

VaultState.sol#L225-L236

  function _convertToShares(
    uint256 assets,
    Math.Rounding rounding
  ) internal view returns (uint256 shares) {
    uint256 totalShares = _totalShares;
    // Will revert if assets > 0, totalShares > 0 and _totalAssets = 0.
    // That corresponds to a case where any asset would represent an infinite amount of shares.
    return
-     (assets == 0 || totalShares == 0)
+     (assets == 0 || totalShares == 0 || _totalAssets == 0)
        ? assets
        : Math.mulDiv(assets, totalShares, _totalAssets, rounding);
  }

hats-finance / StakeWise-0xd91cd6ed6c9a112fdc112b1a3c66e47697f522cd