code-423n4 / 2023-12-ethereumcreditguild-findings

9 stars 5 forks source link

Functions with `onlyCoreRole(CoreRoles.GUARDIAN)` modifier will be unavailable when sequencer is down #1266

Closed c4-bot-4 closed 5 months ago

c4-bot-4 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/GuildGovernor.sol#L110-L117

Vulnerability details

Impact

Functions with the onlyCoreRole(CoreRoles.GUARDIAN) modifier may become unavailable if the sequencer is down. This issue arises due to the interaction of the GUARDIAN role, which is operated by a multisig, with Arbitrum's address aliasing feature.

While it is possible for Externally Owned Accounts (EOAs) to avoid address aliasing on transactions submitted to Arbitrum's delayed inbox, the multisig address will still be aliased when interacting with the L2. As a result, the multisig may not be able to pause the protocol, especially during a network outage. This could potentially leave the protocol and its users unprotected in such scenarios.

The affected functions are pause() and unpause() in CoreRef.sol, and guardianCancel() in GuildGovernor.sol.

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/GuildGovernor.sol#L110-L117 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/core/CoreRef.sol#L57-L64

Proof of Concept

Consider a scenario where the network is experiencing an outage. In this situation, the GUARDIAN role, operated by a multisig, attempts to pause the protocol by calling the pause() function. However, due to Arbitrum's address aliasing, the multisig's address is aliased, and the call to pause() fails, leaving the protocol and its users exposed.

Tools Used

Manual review

Recommended Mitigation Steps

Check for aliased addresses in the onlyCoreRole modifier as recommended in the Arbitrum documentation.

Assessed type

Other

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as remove high or low quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as duplicate of #1253

Trumpero commented 5 months ago

Invalid. I don't see any relation between the multisig and the address alias in this case. The address alias on Arbitrum is used to obtain the address of the sender on L1, while the GUARDIAN role is an Arbitrum address that can execute the pause operation.

c4-judge commented 5 months ago

Trumpero marked the issue as not a duplicate

c4-judge commented 5 months ago

Trumpero marked the issue as unsatisfactory: Invalid