StorageUpgradeableProxy Manual Review Findings

SUP-01M: Incorrect Implementation of Initialization

Type	Severity	Location
Logical Fault		StorageUpgradeableProxy.sol:L43-L45

Description:

The StorageUpgradeableProxy::constructor contains an invalid initialization methodology using delegatecall to self, resulting a "successful" call that does not initialize the contract.

Impact:

The flaw arises from the fact that when the constructor of the contract is being executed its code has not yet been stored on the blockchain. As such, a self-call of any kind (delegatecall / call / staticcall) will be performed to an address with no code thus succeeding while not executing any statement.

Example:

Copy
29/// @dev Initializes the upgradeable proxy with an initial implementation specified by `_target`.
30/// @param _owner Address of proxy owner
31/// @param _storage Address of the storage contract
32/// @param _slot Slot of the implementation
33/// @param _initialCall Optional initial calldata
34constructor(
35  address _owner,
36  address _storage,
37  bytes32 _slot,
38  bytes memory _initialCall
39) {
40  _setImplementation(_storage, _slot);
41  _initializeOwnership(_owner);
42
43  if (_initialCall.length > 0) {
44    CallLib.delegateCallFunc(address(this), _initialCall);
45  }
46}

Recommendation:

We advise the code to perform a delegatecall instruction directly to the _target as otherwise the contract will not be initialized during deployment.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The contract now properly performs a delegatecall instruction to the intended target of the storage which is queried safely via the StorageUpgradeableProxy::implementation function.