Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sherlock-admin4 opened this issue Sep 21, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Sep 21, 2024

branch_indigo

Medium

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 on protocolFeeRecevier 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-admin4 sherlock-admin4 changed the title Tangy Raisin Woodpecker - Invalid protocolFeeReceiver check whenever the global protocolFeeReceiver address is reset branch_indigo - Invalid protocolFeeReceiver check whenever the global protocolFeeReceiver address is reset Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant