Invalid protocolFeeReceiver check whenever the global protocolFeeReceiver address is reset

Summary

Invalid protocolFeeReceiver check whenever the global protocolFeeReceiver address is reset, risk of compromised protocolFeeReceiver address takes protocol fees for all deployed vaults.

Vulnerability Detail

Verifying protocol role address is crucial in securing protocol funds. Current LidoVault.sol’s check of protocolFeeReceiver is essentially invalid whenever the global protocolFeeReceiver address is updated in VaultFactory.sol. It allows an old or potentially compromised address to continue withdrawing protocol fees.

protocolFeeReceiver is stored locally in LidoVault.sol and can never be updated even though the factory updated the global protocolFeeReceiver. See the snippet below.

An edge case is if the previous protocolFeeReceiver is compromised. Admin for vaultFactory.sol will have to change protocolFeeRecevier to a new address through setProtocolFeeReceiver().

The problem is LidoVault.sol ‘s check onprotocolFeeRecevier is vulnerable and it doesn’t call VaultFactory to determine the correct protocolFeeRecevier. It will still use the compromised protocolFeeReceiver address throughout the lifetime of the vault.

This allows the compromised protocolFeeReceiver to steal all the protocol fees in all deployed vaults.

Impact

manual

Code Snippet

//contracts/LidoVault.sol
  function withdraw(uint256 side) external {
...
          //@audit protocolFeeReceiver is stored locally in LidoVault.sol and can never be updated even though the factory updated the global protocolFeeReceiver
 |>      if (msg.sender == protocolFeeReceiver && appliedProtocolFee > 0) {
          require(variableToVaultOngoingWithdrawalRequestIds[msg.sender].length == 0, "WAR");
          return protocolFeeReceiverWithdraw();
        }
...

(https://github.com/sherlock-audit/2024-08-saffron-finance/blob/38dd9c8436db341c331f1b14545770c1766fc0ee/lido-fiv/contracts/LidoVault.sol#L514)

Tool used

Manual Review

Recommendation

In LidoVault.sol, (1) Store factory address during initialize() (2) Call vaultFactory::protocolFeeReceiver when checking protocolFeeReceiver address

sherlock-audit / 2024-08-saffron-finance-judging

branch_indigo - Invalid protocolFeeReceiver check whenever the global protocolFeeReceiver address is reset #149