UtilityFacet Manual Review Findings

UFT-01M: Inexistent Validation of Contract Existence

Type	Severity	Location
Input Sanitization		UtilityFacet.sol:L25

Description:

The UtilityFacet::state function permits a delegatecall operation to be made on an arbitrary address without validating that the contractAddress is indeed part of the Diamond's facets.

Impact:

This exhibit pertains to a centralization risk which does not fall under the normal severity rubrics and is by default assigned a value of unknown.

Example:

Copy
19function state(bytes memory data_) external {
20    IUtility.Subroutine[] memory subroutines = abi.decode(data_, (IUtility.Subroutine[]));
21
22    for (uint256 i = 0; i < subroutines.length; i++) {
23        // callParams is abi.encodeWithSignature("state(bytes)", _bytes)
24        LibDiamond.initializeDiamondCut(
25            subroutines[i].contractAddress,
26            subroutines[i].callParams
27        );
28    }
29}

Recommendation:

We advise the contractAddress of each Subroutine to be properly evaluated as being a member of the Diamond, disallowing delegatecall operations to arbitrary parties.

Alleviation (98768eaf85e712e7881a7349e4e32a1b78fb8591):

The LibDiamond contract was extensively updated to track which facets have been added to the AllianceBlock Diamond, thereby permitting the LibDiamond::initializeDiamondCut function ultimately invoked by UtilityFacet::state to ensure the contract being invoked as part of the initialization process is non-zero.

As such, we consider this exhibit fully alleviated.

UFT-02M: Inexistent Direct Invocation Protection

Type	Severity	Location
Logical Fault		UtilityFacet.sol:L19-L29

Description:

The referenced function is meant to act as a form of state initiation yet does not impose any access control on the logic contract's implementation.

Impact:

It is presently possible to kill the UtilityFacet by tricking it into performing a delegatecall operation that results in a selfdestruct instruction, rendering this exhibit to be of medium severity due to its Denial-of-Service capabilities.

Example:

Copy
13/**
14 * @notice Calls the state functions of other diamond facets
15 * @dev This state method is never attached on the diamond.
16 * This method is to be delegatecall-ed from diamondCutGovernableFacet.diamondCut
17 * and takes as parameter the encoded call data for the state methods of any other diamond facets.
18 */
19function state(bytes memory data_) external {
20    IUtility.Subroutine[] memory subroutines = abi.decode(data_, (IUtility.Subroutine[]));
21
22    for (uint256 i = 0; i < subroutines.length; i++) {
23        // callParams is abi.encodeWithSignature("state(bytes)", _bytes)
24        LibDiamond.initializeDiamondCut(
25            subroutines[i].contractAddress,
26            subroutines[i].callParams
27        );
28    }
29}

Recommendation:

We advise the function to be solely accessible via delegatecall instructions by storing the address(this) evaluation to an immutable contract variable and ensuring that address(this) != self when the UtilityFacet::state function is invoked, guaranteeing that the function can only be accessed via delegatecall instructions.

Alleviation (54fd570de24631ca65a7cea022aebe43225a08c7):

Direct call protection was introduced per our recommendation, utilizing a contract-level immutable variable assigned to address(this) during the contract's deployment.