hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

Missing Validation for `_setImplementation` function in `StrategyProxy` and `VirtualRewarderProxy` #61

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xa4455f79f69cf7fa5c1309051a07292197f1c5789e1e4e2a0a11e1ab1c326960 Severity: high

Description: Description\ The upgradeable proxy contracts StrategyProxy and VirtualRewarderProxy lack a critical validation check in the _setImplementation function to ensure that the newImplementation address contains valid contract code. Specifically, the contract does not include the following requirement:

require(newImplementation.code.length > 0);

This check is essential to verify that the address provided for the new implementation is not empty and contains executable code.

Attack Scenario\ An administrator attempts to upgrade the proxy contract to a new implementation address.

If the new implementation address is set to an address without code (e.g., an address with no deployed contract), the proxy will delegate calls to a non-functional address.

This will cause all function calls to the proxy contract to fail, leading to a complete service disruption.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

    function _setImplementation(address newImplementation) private {
++ require(newImplementation.code.length > 0);
        StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation;

Add a validation check in the _setImplementation function for upgradeable proxy contracts StrategyProxy and VirtualRewarderProxyto ensure that upgrades are applied only to valid contracts.

This will prevent potential service disruptions and maintain user trust in the system's stability and security.

0xmahdirostami commented 3 months ago

Proxy is out of scope, also the new implementation is under the responsibility of the admin which is trusted