Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unchecked _srcChainId Vulnerability in BranchBridgeAgent for Cross-Chain Communication #771

Closed
c4-submissions opened this issue Oct 6, 2023 · 9 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-855 grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L578
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L941-L943

Vulnerability details

Description

The Ulysses scope is a complex cross-chain communication system designed to facilitate secure and efficient message passing between chains. It relies on LayerZero for this purpose, enabling interoperability between various blockchains. However, a vulnerability has been identified in the custom integration with LayerZero.

The 'Root' contracts are intended to be deployed on Arbitrum, while 'Branch' contracts are deployed on several Layer 1 (L1) and Layer 2 (L2) networks, including Ethereum, Polygon, Base, and Optimism. The vulnerability stems from the fact that the protocol does not adequately verify the chainId of the source chain when receiving cross-chain messages through BranchBridgeAgent.

This vulnerability can be opportunistically exploited by a malicious actor with selfish motives, allowing them to impersonate a trusted source, such as the RootBridgeAgent on Arbitrum in another LayerZero supported chains. Subsequently, this individual can execute unauthorized actions on the target chain, which may include Ethereum mainnet, Polygon, Base, or Optimism.

Impact

To demonstrate the exploitability of this vulnerability, let's consider a hypothetical scenario:

  • Unauthorized Actions: An opportunistic attacker could exploit this vulnerability to impersonate a trusted source on a different chain by imitating the address of a legitimate RootBridgeAgent from Arbitrum. This would enable them to send cross-chain messages to BranchBridgeAgent on the target chain without adequate verification.

  • Financial Gain: Depending on the nature of the malicious message and the actions it triggers, an attacker might profit financially by depleting the protocol's assets through unauthorized transfers, withdrawals, or other operations.

  • Disruption: In pursuit of personal gain, an opportunistic attacker could potentially disrupt the normal operation of the protocol by sending malicious messages that induce unintended behaviors or errors, thereby causing protocol instability.

Proof of Concept

To demonstrate this vulnerability, let's consider a hypothetical scenario:

  1. Address Mimicry:

    • The attacker identifies a chain supported by Maia (assuming the LayerZero UA supports specific chains) and LayerZero-supported chains, excluding Arbitrum.
    • On this chosen chain, the attacker deploys a contract and manipulates its initialization parameters or constructor logic to craft a resulting contract address that closely mimics the address of the legitimate RootBridgeAgent deployed on Arbitrum.
  2. Crafting a Malicious Message:

    • Armed with the mimicked address of RootBridgeAgent from Arbitrum, the attacker sends a malicious cross-chain message through LayerZero to BranchBridgeAgent on any Maia-supported chain.
  3. Bypassing Security Checks:

    • Exploiting the protocol's limited checks in the custom integration, the attacker's message successfully passes through the security checks.
      function _requiresEndpoint(address _endpoint, bytes calldata _srcAddress) internal view virtual {
          //Verify Endpoint
          if (msg.sender != address(this)) revert LayerZeroUnauthorizedEndpoint();
          if (_endpoint != lzEndpointAddress) revert LayerZeroUnauthorizedEndpoint();
    
          //Verify Remote Caller
          if (_srcAddress.length != 40) revert LayerZeroUnauthorizedCaller();
          // @audit this would be bypassed if chain is not Arbitrum
          if (rootBridgeAgentAddress != address(uint160(bytes20(_srcAddress[20:])))) revert LayerZeroUnauthorizedCaller();
      }
  4. Resulting Damage:

    • Depending on the message's content and operations it triggers, the attacker could execute unauthorized actions on targeted chain, potentially causing financial loss or protocol disruption.

Tools Used

  • Manual review
  • Foundry

Recommended Mitigation Steps

For those genuinely interested in addressing this vulnerability and enhancing the security of the Maia Protocol's custom integration with LayerZero, consider the following steps:

  1. Implement Chain ID Verification:

    • Modify the BranchBridgeAgent contract to include robust verification for the chainId of the source chain when receiving cross-chain messages in lzReceive or pass the _srcChainId to lzReceiveNonBlocking and preform the check. This should ensure that messages are only accepted from legitimate sources on the correct chain.
  2. Align with LayerZero Best Practices:

    • To establish stronger cross-chain message security, adhere to LayerZero's recommended implementation practices. This includes managing trusted remotes with mappings, as demonstrated in the LzApp contract.
    function lzReceive(
        uint16 _srcChainId,
        bytes calldata _srcAddress,
        uint64 _nonce,
        bytes calldata _payload
    ) public virtual override {
        // lzReceive must be called by the endpoint for security
        require(_msgSender() == address(lzEndpoint), "LzApp: invalid endpoint caller");

        bytes memory trustedRemote = trustedRemoteLookup[_srcChainId];
        // if will still block the message pathway from (srcChainId, srcAddress). should not receive message from untrusted remote.
        require(
            _srcAddress.length == trustedRemote.length && trustedRemote.length > 0 && keccak256(_srcAddress) == keccak256(trustedRemote),
            "LzApp: invalid source sending contract"
        );

        //...

Assessed type

Invalid Validation

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 6, 2023
c4-submissions added a commit that referenced this issue Oct 6, 2023
@c4-pre-sort
Copy link

0xA5DF marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Oct 12, 2023
@c4-pre-sort
Copy link

0xA5DF marked the issue as duplicate of #855

@0xA5DF
Copy link

0xA5DF commented Oct 12, 2023

On this chosen chain, the attacker deploys a contract and manipulates its initialization parameters or constructor logic to craft a resulting contract address that closely mimics the address of the legitimate RootBridgeAgent deployed on Arbitrum.

This is the critical part, I doubt the likelihood of this

@c4-judge
Copy link
Contributor

alcueca changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Oct 25, 2023
@c4-judge
Copy link
Contributor

alcueca marked the issue as grade-b

@alcueca
Copy link

alcueca commented Oct 25, 2023

Grade-b because the vulnerability exists, but the report is highly speculative.

@khalidfsh
Copy link

Hi @alcueca

I believe that the inherent risks associated with this vulnerability, in conjunction with its potential impact, warrant a Medium risk rating. Below are the primary reasons supporting my appeal:

  1. Decentralized Nature and Trust Model: The protocol is designed with a decentralized architecture and a trust model that inherently does not trust any Externally Owned Accounts (EOA), including the original EOA deployer. This design principle underscores the significance of ensuring robust verification mechanisms, especially in cross-chain communications.

  2. Lack of Protection for EOA Accounts: Due to the trust model, the protocol does not provide protective measures for any EOA accounts, rendering it crucial to have robust verification mechanisms in place to prevent unauthorized actions.

  3. Unique Position of Original EOA Deployer: The identified vulnerability can only be exploited by the original EOA deployer, showcasing a unique risk scenario. Despite the low likelihood of exploitation, the potential impact is substantial, as the exploit could lead to unauthorized actions on targeted chains, possibly resulting in financial loss or protocol disruption.

  4. Comparative Impact Analysis: The impact of this vulnerability surpasses that of the currently selected high-risk reports. Even with a low likelihood, the severity of potential consequences should not be underestimated.


I respectfully urge a re-evaluation of the grading in light of the above points. The decentralized nature, trust model, and potential impact underscore the need for a more accurate risk assessment. I am more than willing to provide any additional information or engage in further discussions to elucidate any aspect of the identified vulnerability and my analysis.

I appreciate your time and consideration in revisiting this grading and look forward to your feedback.

@alcueca
Copy link

alcueca commented Nov 3, 2023

I have no idea how I considered this report to be anything. It's complete gibberish.

@c4-judge c4-judge added grade-c unsatisfactory does not satisfy C4 submission criteria; not eligible for awards and removed grade-b labels Nov 3, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Nov 3, 2023

alcueca marked the issue as grade-c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-855 grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

6 participants