code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

The `rootChainId` was hardcoded and made Immutable this goes against the layerzero best practices, which states that "Do not hardcode LayerZero chain Ids. Use admin restricted setters instead". #837

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L53 https://github.com/code-423n4/2023-09-maia/blob/main/src/CoreRootRouter.sol#L47

Vulnerability details

Impact

The _performCall and _performFallbackCall function sends messages to the root-chain using hardcoded rootChainId this could lead to messages not being delivered if Layerzero decides to change chain IDs.

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L765C1-L778C6 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L785C1-L795C6

Proof of Concept

Layerzero advice that chain ID should not be hardcoded but "admin restricted setters" should be used instead

https://layerzero.gitbook.io/docs/evm-guides/layerzero-integration-checklist

  1. If Layerzero updates the chain IDs, there is no way to update the rootChainId on the BranchBridgeAgent and CoreRootRouter contracts.
  2. The following functions on the BranchBridgeAgent won't be able to communicate with the root chain; callOutSystem, callOutAndBridge, callOutAndBridge, callOutAndBridgeMultiple, callOutSigned, callOutSignedAndBridge, callOutSignedAndBridgeMultiple, retryDeposit, retrieveDeposit, retrySettlement, lzReceiveNonBlocking and _execute.

Because they call either the _performCall or the _performFallbackCall function that uses the hardcoded rootChainId.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Do not hardcode LayerZero chain Ids. Use admin-restricted setters instead

Assessed type

DoS

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #415

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

alcueca marked the issue as grade-a