CENTRALIZATION RELATED RISKS

[NegruGeorge]

Describe the bug
In the UpgradeL1Bridge contract, the role closer has exclusive authority over several critical functions. Any compromise to the closer account could allow a malicious actor to exploit this authority.

Affected Functions:

• function forceRegistry(address[] calldata _position) external onlyCloser
• function forceModify(ForceRegistryParam[] calldata _data) external onlyCloser
• function forceActive(bool _state) external onlyCloser

Configuration

• Severity: LOW

Impact

• Use forceActive function to stop the contract:
  If an attacker gains control of the closer account, they can deactivate the contract by setting the active state to false. This would disrupt the normal operation of the contract, potentially halting all its functionalities.

• Use forceRegistry. This function registers new storage contract addresses that can be used for forced withdrawals:
  An attacker can register malicious storage contracts. These contracts can be designed to store fraudulent or manipulated data, enabling the attacker to withdraw funds from the bridge contract by exploiting the forceWithdrawClaim and forceWithdrawClaimAll functions.

• Use forceModify, This function modifies the state of already registered storage addresses:
  An attacker can change the state of any registered storage contract, disrupting the normal operation and security of the contract.

Recommendation
To mitigate the risks, it's recommended to implement a multi-signature (multisig) mechanism (e.g., 2/3 or 3/5 multisig) for sensitive operations. This approach can delay the execution of critical operations and avoid a single point of failure.

Exploit Scenario

1. Deactivating the Contract:
   An attacker gains control of the closer account and calls the forceActive function with _state set to false, stopping the contract and halting its functionalities.

2. Registering Malicious Storage Contracts:
   The attacker uses the forceRegistry function to register one or more malicious storage contracts.
   The attacker uses these contracts to store manipulated data that can be used to exploit the withdrawal functions.

3. Modifying Storage Contract States:
   The attacker uses the forceModify function to change the state of any registered storage contract, disrupting the normal operation and security of the contract

Demo

[DevUreak]

That's a really good idea. We will consider further development in this area.

[zzooppii]

@suahnkim What do you think about it?

[suahnkim]

As @zzooppii recommended on slack, I think utilizing Tokamak Network DAO will help with improving the access control for these critical functions.

[DevUreak]

After discussion, we want to use DAO and multisig contract address for "Closer".

Functionally, there is currently nothing to change.

But the issue will not be closed. Because I think I still need to get more feedback.

[DevUreak]

#23