Lack of validation to check whether or not the `msg.value` sent as the security deposit would be more than `1 gwei`, which lead to the inflation attack

[hats-bug-reporter[bot]]

Github username: -- Submission hash (on-chain): 0xa70832e7435e348e7e4431d26361fb2196e5bd3161742ec983e40adc0aa48774 Severity: medium

Description: Title:\ Lack of validation to check whether or not the msg.value sent as the security deposit would be more than 1 gwei, which lead to the inflation attack

Severity:\ Medium

Description:\ Within the EthVaultFactory#createVault(), the EthVault#initialize() would be called with msg.value to initialize a Vault like this: https://github.com/stakewise/v3-core/blob/5996ae760a7e4a24d42029e64c56f3df087053cd/contracts/vaults/ethereum/EthVaultFactory.sol#L58

  /// @inheritdoc IEthVaultFactory
  function createVault(
    bytes calldata params,
    bool isOwnMevEscrow
  ) external payable override returns (address vault) {
    // create vault
    vault = address(new ERC1967Proxy(implementation, ''));

    // create MEV escrow contract if needed
    address _mevEscrow;
    if (isOwnMevEscrow) {
      _mevEscrow = address(new OwnMevEscrow(vault));
      // set MEV escrow contract so that it can be initialized in the Vault
      ownMevEscrow = _mevEscrow;
    }

    // set admin so that it can be initialized in the Vault
    vaultAdmin = msg.sender;

    // initialize Vault
    IEthVault(vault).initialize{value: msg.value}(params); /// @audit 

    // cleanup MEV escrow contract
    if (isOwnMevEscrow) delete ownMevEscrow;

    // cleanup admin
    delete vaultAdmin;

    // add vault to the registry
    _vaultsRegistry.addVault(vault);
    ...

Within the EthVault#initialize(), the Vault would be initialized like this: https://github.com/stakewise/v3-core/blob/5996ae760a7e4a24d42029e64c56f3df087053cd/contracts/vaults/ethereum/EthVault.sol#L70-L76

  /// @inheritdoc IEthVault
  function initialize(bytes calldata params) external payable virtual override initializer {
    __EthVault_init(
      IEthVaultFactory(msg.sender).vaultAdmin(),
      IEthVaultFactory(msg.sender).ownMevEscrow(),
      abi.decode(params, (EthVaultInitParams))
    );
  }

According to the "Parameters" part of creating a Vault in the documentation, 1 gwei must be transferred as a security deposit when the EthVaultFactory#createVault() would be called like this:

The security deposit of 1 gwei must be transferred with the call. This protects the vault stakers from the inflation attack described here.

However, within both the EthVaultFactory#createVault() and the EthVault#initialize() above, there is no validation to check whether or not the msg.value sent when the EthVaultFactory#createVault() is called would be more than 1 gwei.

As a result, the caller (msg.sender) can create a Vault without any security deposit.

This lead to the inflation attack, which is mentioned in the "Parameters" part of creating a Vault in the documentation.

Recommendation:\ Within the EthVaultFactory#createVault(), consider adding a validation to check whether or not the msg.value sent when the EthVaultFactory#createVault() is called would be more than 1 gwei (1e9) like this:

  /// @inheritdoc IEthVaultFactory
  function createVault(
    bytes calldata params,
    bool isOwnMevEscrow
  ) external payable override returns (address vault) {
    // create vault
    vault = address(new ERC1967Proxy(implementation, ''));
    ...

    // set admin so that it can be initialized in the Vault
    vaultAdmin = msg.sender;

+   require(msg.value >= 1e9, "msg.value sent as the security deposit must be more than 1 gwei");

    // initialize Vault
    IEthVault(vault).initialize{value: msg.value}(params);
    ...

[0xmahdirostami]

Hello, https://github.com/stakewise/v3-core/blob/c82fc57d013a19967576f683c5e41900cbdd0e67/contracts/vaults/modules/VaultEthStaking.sol#L31

  uint256 private constant _securityDeposit = 1e9;

https://github.com/stakewise/v3-core/blob/c82fc57d013a19967576f683c5e41900cbdd0e67/contracts/vaults/modules/VaultEthStaking.sol#L141-L147

  function __VaultEthStaking_init() internal onlyInitializing {
    __ReentrancyGuard_init();

    // see https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3706
    if (msg.value < _securityDeposit) revert Errors.InvalidSecurityDeposit();
    _deposit(address(this), msg.value, address(0));
  }