Description

During the construction of the VaultFactory, the implementation LidoVault is constructed:

constructor(uint256 _protocolFeeBps, uint256 _earlyExitFeeBps) {
  // [...]

  vaultContract = address(new LidoVault());
}

However, this implementation contract is fully usable by the first to call initialize on it, due to a lack of a constructor that disables the initializer.

Impact

The only impact is that if that contract is initialized maliciously, it can be used once to charge higher-than-expected fees and send the fees to an incorrect protocol-fee receiver. Although the view function wasDeployedByFactory will return false for this vault, on Etherscan and similar chain explorers, the contract will still say it was deployed by the vault, which can mislead users.

Recommendations

Even though the impact is small, we recommend adding a constructor to the LidoVault that explicitly disables the implementation contract from being used.

Remediation

This issue has been acknowledged by Saffron, and fixes were implemented in the following commits:

• 6c498b48↗
• 4877d7fa↗
• 80930eeb↗