ClientExternalAddressesBase Static Analysis Findings

CEA-01S: Incorrect payable Specifier

Type	Severity	Location
Language Specific		ClientExternalAddressesBase.sol:L31

Description:

The constructor of the ClientExternalAddressesBase contract is set as payable incorrectly.

Impact:

Funds can currently be permanently locked in the contract in case they are sent during its construction.

Example:

Copy
27constructor(
28    address _accessController,
29    address _protocolAddress,
30    address _impl
31) payable {
32    // Get the ProxyStorage struct
33    ClientLib.ProxyStorage storage ps = ClientLib.proxyStorage();
34
35    // Store the AccessController address
36    ps.accessController = IAccessControlUpgradeable(_accessController);
37
38    // Store the Protocol Diamond address
39    ps.protocolDiamond = _protocolAddress;
40
41    // Store the implementation address
42    ps.implementation = _impl;
43}

Recommendation:

We advise the payable modifier to be omitted avoiding lock of funds.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

The payable specifier from the constructor has been properly omitted from the codebase as advised.

CEA-02S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		ClientExternalAddressesBase.sol:L27-L43, L60-L69, L85-L94, L116-L125

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
27constructor(
28    address _accessController,
29    address _protocolAddress,
30    address _impl
31) payable {
32    // Get the ProxyStorage struct
33    ClientLib.ProxyStorage storage ps = ClientLib.proxyStorage();
34
35    // Store the AccessController address
36    ps.accessController = IAccessControlUpgradeable(_accessController);
37
38    // Store the Protocol Diamond address
39    ps.protocolDiamond = _protocolAddress;
40
41    // Store the implementation address
42    ps.implementation = _impl;
43}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each address specified is non-zero.

Alleviation (44009967e4):

The now-two input arguments are correctly sanitized in the first referenced instance (the constructor), however, the remaining referenced instances remain unaddressed rendering this exhibit partially dealt with.

Alleviation (6dae5d2602):

Input sanitization has now been introduced to the setImplementation and setProtocolAddress functions whilst the setAccessController function still allows a zero-value input address which is intended functionality based on the Boson Protocol team's response. As a result, we consider this exhibit fully alleviated.