Manager Static Analysis Findings

MAN-01S: Arbitrary Code Execution

Type	Severity	Location
Language Specific		Manager.sol:L208

Description:

The linked statement performs arbitrary code execution that could even lead to the contract deleting itself.

Example:

Copy
205function _executeControllerCommand(ControllerTransferData calldata transfer) private {
206    address controllerAddress = registeredControllers[transfer.controllerId];
207    require(controllerAddress != address(0), "INVALID_CONTROLLER");
208    controllerAddress.functionDelegateCall(transfer.data, "CYCLE_STEP_EXECUTE_FAILED");
209    emit DeploymentStepExecuted(transfer.controllerId, controllerAddress, transfer.data);
210}

Recommendation:

We advise the code to be revised as it should utilize explicit function calls rather than delegate instructions. Alternatively, we advise the code to be revised to utilize a different contract that delegate calls to prevent the parent one from ever being susceptible to deletion as it can significantly affect the protocol's perceived value for compromisation.

Alleviation:

The Olive Protocol team considered this exhibit but opted not to apply a remediation for it in the current iteration of the codebase.

MAN-02S: Inexistent Validation of Input Address

Type	Severity	Location
Input Sanitization		Manager.sol:L122-L124

Description:

The linked function accepts an address input argument yet does not properly sanitize it.

Example:

Copy
122function setStaking(address _staking) external override onlyAdmin {
123    staking = _staking;
124}

Recommendation:

We advise it to be validated against the zero-address to prevent misconfiguration of the contract.

Alleviation:

The setStaking function has been entirely omitted from the codebase thereby nullifying this exhibit.

MAN-03S: Inefficient Usage of EnumerableSet

Type	Severity	Location
Gas Optimization		Manager.sol:L91, L93, L98, L101, L105, L106, L111, L112

Description:

The add and remove instructions of the EnumerableSet yield a bool variable signaling whether the operation was executed successfully which can be utilized in place of the contains checks.

Example:

Copy
90function registerController(bytes32 id, address controller) external override onlyAdmin {
91    require(!controllerIds.contains(id), "CONTROLLER_EXISTS");
92    registeredControllers[id] = controller;
93    controllerIds.add(id);
94    emit ControllerRegistered(id, controller);
95}
96
97function unRegisterController(bytes32 id) external override onlyAdmin {
98    require(controllerIds.contains(id), "INVALID_CONTROLLER");
99    emit ControllerUnregistered(id, registeredControllers[id]);
100    delete registeredControllers[id];
101    controllerIds.remove(id);
102}
103
104function registerPool(address pool) external override onlyAdmin {
105    require(!pools.contains(pool), "POOL_EXISTS");
106    pools.add(pool);
107    emit PoolRegistered(pool);
108}
109
110function unRegisterPool(address pool) external override onlyAdmin {
111    require(pools.contains(pool), "INVALID_POOL");
112    pools.remove(pool);
113    emit PoolUnregistered(pool);
114}

Recommendation:

We advise this to be done so to optimize the codebase's gas cost.

Alleviation:

All linked instances have been refactored to properly utilize the returned bool of the addition and removal operations on the EnumerableSet.