vlad - Unprotected initialize function of the implementation contract

[sherlock-admin]

vlad

unlabeled

Unprotected initialize function of the implementation contract

Summary

There is no protection on calling initialize function on the implementation contract.

Severity

Medium

Vulnerability Detail

There is no protection from calling initialize function on the implementation contracts ContractWhitelist and HardenedTopupProxy. So anyone can call initialize on it with custom parameters. In case when implementation contract logic uses delegatecall instruction relying on "safe" initialization process parameters attacker has the possibility of substitution of delegatecall parameter by providing an address of the custom contract that destroys it using selfdestuct opcode.

Impact

In case of an upgrade, implementation can contain logic that uses delegatecall (as an example, delegatecall is allowed only to the address that is provided by trusted admin during initialization). In such case, an attacker will have the ability to initialize the implementation contract with custom parameters and make it call (through a delegatecall) code that destroys itself with selfdestruct opcode. You can read more about similar vulnerability here.

Code Snippet

• ContractWhitelist.sol initialize function
• HardenedTopupProxy.sol initialize function

Tool used

Manual Review

Recommendation

Consider adding a special check that the initialize function is not called on the implementation contract. As an example, you can use the following pattern:

address private immutable __self = address(this);

modifier onlyProxyCall() {
    require(address(this) != __self, "Function must be called through delegatecall");
    _;
}

function initialize(...) public onlyProxyCall initializer ...