sherlock-audit / 2023-02-blueberry-judging

12 stars 5 forks source link

eierina - Avoid leaving a contract uninitialized #341

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

eierina

false

Avoid leaving a contract uninitialized

Summary

Logic contracts are left uninitialized.

Raising as a low priority as it seems there are no risks of having the logic destroyed, but there could be in future impementations.

Vulnerability Detail

There are multiple contracts in the project that are deployed as proxies to allow for easy upgrades. When a proxy is created, a deployment script is used to call a method that initializes important functionalities, such as setting the owner of the contract. This method is protected by an initializer modifier, so it cannot be called again afterwards.

However, the issue is that the initialization happens in the proxy state context and therefore the initialize method is never called on the implementation contract itself. This means that an attacker could potentially call the initializer method and take ownership of the implementation contract.

Raising as a low priority there seems to be no direct or indirect access to selfdestruct() that would allow to destroy the logic contract, but there could be in future impementations.

Impact

Logic contracts can be initialized by an attacker.

Code Snippet

The following contracts require initializers disabled in the constructor.

https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/BlueBerryBank.sol#L96 https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/ProtocolConfig.sol#L28 https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/oracle/CoreOracle.sol https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/spell/IchiVaultSpell.sol https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/vault/HardVault.sol https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/vault/SoftVault.sol https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/wrapper/WERC20.sol https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/wrapper/WIchiFarm.sol

Tool used

Manual Review

Recommendation

Add _disableInitializers() the the constructors of upgradeable contracts to avoid leaving initialize function of logic contracts invokable by attackers.

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
    _disableInitializers();
}