Single-step process for critical ownership transfer is very risky

[hats-bug-reporter[bot]]

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xbb556fd43965a09d722cb75714e7161ab618caa094f8b827fb7c197e174c4ede Severity: low

Description: Vulnerability Details

Single-step process for critical ownership transfer is risky due to possible human error which could result in locking all the functions that use the onlyOwner modifier in BribeFactoryUpgradeable.sol

contract BribeFactoryUpgradeable is IBribeFactory, BlastGovernorSetup, OwnableUpgradeable {

BribeFactoryUpgradeable.sol inherits openzeppelins OwnableUpgradeable .sol which is not safe as the process is 1-step for transfer of ownership which is very risky due to a possible human error and such an error is unrecoverable.

For example, an incorrect address, for which the private key is not known, could be passed accidentally.

In BribeFactoryUpgradeable.sol functions using onlyOwner modifier like changeImplementation(), setVoter(), setDefaultBlastGovernor(), addRewards(), pushDefaultRewardToken(), removeDefaultRewardToken(), etc will be locked and can not be used if the owner address is set incorrectly and in worst case the whole BribeFactoryUpgradeable.solcontract will be of no use if such critical functions can not be accessed by real owner.

Recommendations

Use Ownable2StepUpgradeable.sol instead of OwnableUpgradeable.sol

- import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+ import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";
import {BribeProxy} from "./BribeProxy.sol";
import {IBribe} from "./interfaces/IBribe.sol";
import {IBribeFactory} from "./interfaces/IBribeFactory.sol";
import {BlastGovernorSetup} from "../integration/BlastGovernorSetup.sol";

- contract BribeFactoryUpgradeable is IBribeFactory, BlastGovernorSetup, OwnableUpgradeable {
+ contract BribeFactoryUpgradeable is IBribeFactory, BlastGovernorSetup, Ownable2StepUpgradeable {

Note:

Other contracts like MinterUpgradeable.sol have used Ownable2StepUpgradeable.sol, Therefore all such contracts should follow same two step ownership transfer pattern.

[0xRizwan]

instance 2 of this issue in Fenix.sol at L-18

instance 3 of this issue in MerklGaugeMiddleman .sol at L-23

instance 4 of this issue in GaugeFactoryUpgradeable .sol at L-11

[BohdanHrytsak]

Thank you for the submission.

This submission is under Centralized Risk & Misconfiguration. OOS