Description

The LidoVault contract contains state variables that are implicitly initialized to constant values:

contract LidoVault is ILidoVaultInitializer, ILidoVault {
  // [...]

  /// @notice Minimum amount of ETH that can be deposited for variable or fixed side users
  uint256 public minimumDepositAmount = 0.01 ether;

  /// @notice Minimum amount of the fixed capacity that can be deposited for fixed side users on a single deposit in basis points
  uint256 public minimumFixedDepositBps = 500; // default 5%

  // [...]
}

However, in VaultFactory, a new LidoVault is deployed using Clones.clone:

function createVault(
  uint256 _fixedSideCapacity,
  uint256 _duration,
  uint256 _variableSideCapacity
) public virtual {
  // Deploy vault (Note: this does not run constructor)
  address vaultAddress = Clones.clone(vaultContract);
  
  // [...]
}

This means that the state variables mentioned above are uninitialized in the clone.

Impact

For all vaults deployed by this factory, the minimum deposit amount and minimum fixed basis points are uninitialized and zero, effectively turning off those two limits. This can lead to vaults being created with incorrect incentives, dusting attacks on vaults, and everything else those limits were intended to guard against.

Recommendations

Make these constants constant or immutable so that they are not assigned a storage slot, so that they cannot be erroneously uninitialized in clones.

Additionally, we recommend having the test fixtures deploy the LidoVault that is tested through the LidoVaultFactory, rather than directly. That is why this issue was missed in the tests - in deployVault in the file 1.LidoVault.test.ts, even though it calls the variable LidoVaultFactory, it is actually a LidoVault:

let LidoVaultFactory = await ethers.getContractFactory('LidoVault')

Remediation

This issue has been acknowledged by Saffron, and a fix was implemented in commit eb7fbd37↗.