BeaconManager Manual Review Findings

BMR-01M: Inexistent Initialization of Access Control / Ownable

Type	Severity	Location
Language Specific		BeaconManager.sol:L12

Description:

The contract inherits the upgradeable counterparts of OwnableUpgradeable and AccessControlUpgradeable yet does not initialize them.

Example:

Copy
12contract BeaconManager is OwnableUpgradeable, AccessControlUpgradeable {

Recommendation:

We advise them to be properly initialized by the contract to ensure its role-guarded functions can be properly invoked. Alternatively, if the contract is meant to be inherited by others that initialize those instances it should be marked as abstract denoting that it is not to be deployed independently.

Alleviation (0ed41ccc18a72b7e559b8d79ab7ba6172362ee3b):

The BeaconManager contract has been marked as abstract thus alleviating this exhibit given that its dependencies should be initialized by child contracts instead.

BMR-02M: Inexistent Initialization of Base Implementation

Type	Severity	Location
Language Specific		BeaconManager.sol:L12

Description:

The contract does not properly initialize the base logic implementation permitting it to be taken over by a malicious party.

Impact:

While not an active security threat, it can evolve into one if any form of delegatecall capability is introduced in one of the dependencies of the contract that could cause it to invoke a selfdestruct instruction.

Example:

Copy
12contract BeaconManager is OwnableUpgradeable, AccessControlUpgradeable {

Recommendation:

We advise a constructor to be introduced that simply invokes the initializer modifier to ensure that the logic implementation cannot be initialized maliciously.

Alleviation (0ed41ccc18a72b7e559b8d79ab7ba6172362ee3b):

The BeaconManager contract has been marked as abstract thus rendering the presence of a constructor incorrect and nullifying this exhibit as part of the remediation process for BMR-01M.

BMR-03M: Insufficient Data Deletion

Type	Severity	Location
Logical Fault		BeaconManager.sol:L99-L105

Description:

The deregisterBeacon function does not properly clear up the beaconTypes data entry.

Impact:

External protocols that assume beaconTypes reflects a beacon's registration to the system will be misled if a beacon is deregistered as that data point is not properly updated.

Example:

Copy
96/// @dev Removes a beacon associated with a vault type
97/// @param _name The name of the beacon (identifier)
98/// @dev This will stop the creation of more vaults of the type provided
99function deregisterBeacon(string calldata _name)
100    external
101    onlyRole(BEACON_CREATOR)
102{
103    emit BeaconDeregistered(_name);
104    delete beaconAddresses[_name];
105}

Recommendation:

We advise it to be properly deleted as otherwise stale data can confuse protocol integrators who rely on them.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

After discussion with the Steer Protocol team, we have concluded that the beacon type entries are essential for off-chain processes and should not be deleted. As such, we consider this exhibit nullified.