updating blast governor will not sync to already deployed contract, due to `BlastGovernorSetup` lack of update governor function

Github username: -- Twitter username: -- Submission hash (on-chain): 0x448a641d442228e8669be554a6eec35dd2be3b377f9f984e891cc4f6c7f32b22 Severity: medium

Description: Description

In some contracts, such as BribeFactoryUpgradeable, PairFactoryUpgradeable, GaugeFactoryUpgradeable, FeesVaultFactory, AlgebraFactory, there is a function setDefaultBlastGovernor.

This function is intended to update defaultBlastGovernor, and there is no check if the defaultBlastGovernor is already assign or still empty. If the defaultBlastGovernor is designed to only be assign once, then there should be a check, such as require(defaultBlastGovernor == address(0), "already set");.

    function setDefaultBlastGovernor(address defaultBlastGovernor_) external virtual onlyOwner {
        _checkAddressZero(defaultBlastGovernor_);

        emit SetDefaultBlastGovernor(defaultBlastGovernor, defaultBlastGovernor_);
        defaultBlastGovernor = defaultBlastGovernor_;
    }

Thus, by this assumption, there is a possible case the existing defaultBlastGovernor will be replaced or updated into a new address.

In many contract instance which inherits BlastGovernorSetup, the case when the blast governor need to be updated, there is no available function to update this. For example, in BribeUpgradable the change of blast gov is only through __BlastGovernorSetup_init(governor_); in initialize. There is no way to change this governor again.

Whenever the Bribe Contract Factory change it's defaultBlastGovernor, then the already deployed (Bribe) contract will not be able to update this new blast gov, because lack of access to trigger the __BlastGovernorSetup_init (as it's an internal function in BlastGovernorSetup)

File: BlastGovernorSetup.sol
12: abstract contract BlastGovernorSetup {
...
24:     function __BlastGovernorSetup_init(address gov_) internal {
25:         if (gov_ == address(0)) {
26:             revert AddressZero();
27:         }
28:         IBlast(0x4300000000000000000000000000000000000002).configureGovernor(gov_);
29:     }
30: }

For example, Fenix contract as follow,

File: Fenix.sol
18: contract Fenix is IFenix, BlastGovernorSetup, ERC20Burnable, Ownable {
...
25:     constructor(address blastGovernor_, address minter_) ERC20("Fenix", "FNX") Ownable() {
26:         __BlastGovernorSetup_init(blastGovernor_);
27:         _mint(msg.sender, 7_500_000e18);
28:         _transferOwnership(minter_);
29:     }
...
43:     function mint(address to_, uint256 amount_) external virtual override onlyOwner {
44:         _mint(to_, amount_);
45:     }
46: }
47:

here the blastGovernor_ is more likely an immutable value, because the blastGovernor_ is fixed. But, in BribeFactoryUpgradeable, the _blastGovernor can be updated, which make sense since the Bribe instance will be created using this defaultBlastGovernor.

File: BribeFactoryUpgradeable.sol
010: contract BribeFactoryUpgradeable is IBribeFactory, BlastGovernorSetup, OwnableUpgradeable {
...
014:     address public defaultBlastGovernor;
...
021:     function initialize(address _blastGovernor, address _voter, address _bribeImplementation) external initializer {
022:         __BlastGovernorSetup_init(_blastGovernor);
...
027:         defaultBlastGovernor = _blastGovernor;
...
030:     }
...
078:     function setDefaultBlastGovernor(address defaultBlastGovernor_) external virtual onlyOwner {
079:         _checkAddressZero(defaultBlastGovernor_);
080: 
081:         emit SetDefaultBlastGovernor(defaultBlastGovernor, defaultBlastGovernor_);
082:         defaultBlastGovernor = defaultBlastGovernor_;
083:     }
...
142: }

But my issue is, when the blast governor is being changed due to some issue, the deployed Bribes and Fenix contract are unable to update the governor address.

Attack Scenario

When there is need to change default blast governor, as the function allowed for changing this value, then the already deployed contracts will use the old governor. The issue will be came serious when the change of this default blast governor is compromised, and there is no way to change the gov.

Attachments

Proof of Concept (PoC) File
Revised Code File (Optional)

for example, we use a IBribeFactory reference, in

File: BlastGovernorSetup.sol
12: abstract contract BlastGovernorSetup {
...
+       address contractFactory;
24:     function __BlastGovernorSetup_init(address gov_) internal {
25:         if (gov_ == address(0)) {
26:             revert AddressZero();
27:         }
28:         IBlast(0x4300000000000000000000000000000000000002).configureGovernor(gov_);
29:     }
...
+       function updateGovernor() external { 
+           // if instance from factory, get the `defaultBlastGovernor` from the factory
+           // if not a factory instance, then use the modifier check for owner of contract inherits
+           // this BlastGovernorSetup, and set defaultBlastGovernor with a call to this `updateGovernor`
+           __BlastGovernorSetup_init(defaultBlastGovernor);
+       }
30: }

and modify

    function setDefaultBlastGovernor(address defaultBlastGovernor_) external virtual onlyOwner {
        _checkAddressZero(defaultBlastGovernor_);

        emit SetDefaultBlastGovernor(defaultBlastGovernor, defaultBlastGovernor_);
        defaultBlastGovernor = defaultBlastGovernor_;
+       updateGovernor();
    }

Recommendation

Consider to implement an external function to reconfigure blast governor, and trigger the call on setDefaultBlastGovernor for example.

hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

updating blast governor will not sync to already deployed contract, due to `BlastGovernorSetup` lack of update governor function #52