Liquid Staking

Summary

The VaultControllerStrategy contract uses delegatecall to interact with the vaultDepositController in its deposit and withdraw functions without strict validation of the vaultDepositController address. Since delegatecall executes code in the context of the calling contract, this can lead to execution of arbitrary code if the vaultDepositController address is compromised or malicious, potentially resulting in loss of funds or unexpected behavior.

Vulnerability Details

In the VaultControllerStrategy contract, the deposit and withdraw functions use delegatecall to call functions from the vaultDepositController contract:

The use of delegatecall means that the code at the vaultDepositController address is executed in the context of the VaultControllerStrategy contract, with access to its storage and balance. If the vaultDepositController is set to an unintended or malicious contract, it could execute arbitrary code, modify the storage variables, or transfer funds.

The vaultDepositController address can be updated via the setVaultDepositController function:

While this function is restricted to the contract owner, if the owner's private key is compromised, or if the owner acts maliciously, they could set the vaultDepositController to a malicious contract.

Impact

• Loss of Funds: A malicious vaultDepositController could drain funds from the VaultControllerStrategy contract.

• Unauthorized Access: An attacker could manipulate internal state variables, leading to potential loss or mismanagement of funds.

• Complete Contract Compromise: The entire VaultControllerStrategy could be compromised, affecting all users and associated funds.

Tools Used

Manual code review.

Recommendations

• Limit the ability to change the vaultDepositController address to a multi-signature wallet or a well-audited governance mechanism.

  Before performing delegatecall, validate that the vaultDepositController address is a valid contract using Address.isContract.

  Consider making the vaultDepositController address immutable if upgrades are not necessary.