Reach out for an audit or to learn more about Macro
or Message on Telegram

Connext A-7

Security Audit

July 3rd, 2024

Version 1.0.0

Presented by 0xMacro

Table of Contents

Introduction

This document includes the results of the security audit for Connext's smart contract code as found in the section titled ‘Source Code’. The security audit was performed by the Macro security team between June 11th and June 14th, 2024.

The purpose of this audit is to review the source code of certain Connext Solidity contracts, and provide feedback on the design, architecture, and quality of the source code with an emphasis on validating the correctness and security of the software in its entirety.

Disclaimer: While Macro’s review is comprehensive and has surfaced some changes that should be made to the source code, this audit should not solely be relied upon for security, as no single audit is guaranteed to catch all possible bugs.

Overall Assessment

The following is an aggregation of issues found by the Macro Audit team:

Severity Count Acknowledged Won't Do Addressed
Critical 1 - - 1
Medium 2 - 2 -
Low 1 - - 1
Code Quality 3 - - 3
Informational 2 - - 2

Connext was quick to respond to these issues.

Specification

Our understanding of the specification was based on the following sources:

Source Code

The following source code was reviewed during the audit:

Specifically, we audited the following contract within this repository:

Contract SHA256
src/L1ArbitrumEnabledXERC20.sol

683894fa61b0e8f267965a33fb1b243134c6b588fb3ad1bc507fef13b2f870fc

src/L1LockboxGateway.sol

86b983afa049161efe2341a48c438fadf8c77160b6550e9039d698645904d9c3

src/L1XERC20Adapter.sol

942f4e6a52d9f220880adcac0b2a1c169aabc3a9e258624d0ef8f651c439ecf0

src/L1XERC20Gateway.sol

153133462821096ad233a4a46a4b4a480daf913817e2fabc754b3b242fb49350

src/L2ArbitrumEnabledXERC20.sol

f996733307eb5860ca5ce83ce2bb8529b68f45e2dae2ff08373fbf360a33a763

src/L2LockboxGateway.sol

758dbd1d507499a34a9dbfdfdf5f3151885db933c5749f4c6f9c581471a9d46d

src/L2XERC20Gateway.sol

b2c2fcfcda40d5daace56eafc1631e046c39d960027afff2d0c3bb349054e156

src/XERC20BaseAdapter.sol

74e717a9f767ef5f982ff5b7e2dc40575f28011be4225052dc2f928a5f112c36

src/XERC20BaseGateway.sol

6db9b89db6f313075811e8da27aa80402b252d80a9a613b30b50dca49109024f

src/libraries/L1ArbitrumEnabled.sol

e5b95030022142b9ee6c2c0d3252c33239b6db7fe7811a2342a666db1093b101

src/vendor/L2ArbitrumGateway.sol

5a236700be7a34740cdb8a5a23112246777b0ca351633485249d23b0006c6f18

src/vendor/L2CustomGateway.sol

9a191fd685bf8487cd89da0557679ab2e4b24281cd168606bfa4e91b4a86056d

Note: This document contains an audit solely of the Solidity contracts listed above. Specifically, the audit pertains only to the contracts themselves, and does not pertain to any other programs or scripts, including deployment scripts.

Issue Descriptions and Recommendations

Click on an issue to jump to it, or scroll down to see them all.

Security Level Reference

We quantify issues in three parts:

  1. The high/medium/low/spec-breaking impact of the issue:
    • How bad things can get (for a vulnerability)
    • The significance of an improvement (for a code quality issue)
    • The amount of gas saved (for a gas optimization)
  2. The high/medium/low likelihood of the issue:
    • How likely is the issue to occur (for a vulnerability)
  3. The overall critical/high/medium/low severity of the issue.

This third part – the severity level – is a summary of how much consideration the client should give to fixing the issue. We assign severity according to the table of guidelines below:

Severity Description
(C-x)
Critical

We recommend the client must fix the issue, no matter what, because not fixing would mean significant funds/assets WILL be lost.

(H-x)
High

We recommend the client must address the issue, no matter what, because not fixing would be very bad, or some funds/assets will be lost, or the code’s behavior is against the provided spec.

(M-x)
Medium

We recommend the client to seriously consider fixing the issue, as the implications of not fixing the issue are severe enough to impact the project significantly, albiet not in an existential manner.

(L-x)
Low

The risk is small, unlikely, or may not relevant to the project in a meaningful way.

Whether or not the project wants to develop a fix is up to the goals and needs of the project.

(Q-x)
Code Quality

The issue identified does not pose any obvious risk, but fixing could improve overall code quality, on-chain composability, developer ergonomics, or even certain aspects of protocol design.

(I-x)
Informational

Warnings and things to keep in mind when operating the protocol. No immediate action required.

(G-x)
Gas Optimizations

The presented optimization suggestion would save an amount of gas significant enough, in our opinion, to be worth the development cost of implementing it.

Issue Details

C-1

XERC20 on L1 and L2 can be stolen up to the max mint limit

Topic
Spec
Status
Impact
High
Likelihood
High

The L1XERC20Gateway contract allows permissionless registration of multiple token pairs meant to be supported for bridging. L1XERC20Gateway instance is allowed to mint and burn XERC20 tokens according to preconfigured limits. L1XERC20Gateway is configured with a call to registerTokenOnL2 on an instance of the L1ArbitrumEnabledXERC20 contract if XERC20 is just to be created or on an instance of the L1XERC20Adapter contract if the XERC20 token already exists. L1XERC20Adapter, even though a separate contract with a different contract address acts as a stand-in for the XERC20 token. L1XERC20Gateway, when interacting with the instance of L1XERC20Adapter, will retrieve a reference to the underlying XERC20 token and perform corresponding operations (mint/burn) directly with the XERC20 token.

A malicious attacker may exploit the current characteristics of the system to mint an unbacked amount of a particular XERC20 token up to the limit allowed for a particular gateway instance. To achieve this attacker may perform the following steps:

  1. L1XERC20Gateway is configured (registerTokenOnL2 was called on A):

    • l1Token: A - which is an instance of L1ArbitrumEnabledXERC20 or L1XERC20Adapter
    • l2Token: B - which is an instance of L2ArbitrumEnabledXERC20
  2. Attacker creates 2 contracts:

    • maliciousL1Token: C - which is an instance of L1XERC20Adapter for the same XERC20 used in contract A.
    • maliciousL2Token: D - which is an instance of L2ArbitrumEnabledXERC20 and under full control of the attacker.
  3. The attacker registers the C/D token pair with the same L1XERC20Gateway that has the A/B token pair already registered.

    • As a result of successful registration, registerTokenFromL1() is processed on the instance of L2XERC20Gateway, and l1ToL2Token mapping is updated to contain a C/D value pair.
  4. Attacker mints/obtains an amount of token D up to the specified limit of L1XERC20Gateway for minting XERC20 referenced in token A.

  5. The attacker invokes L2XERC20Gateway:outboundTransfer() for token C, resulting in the provided amount of token D being burned, and withdrawal is triggered on L1.

  6. On L1 in L1ArbitrumGateway:finalizeInboundTransfer(), withdrawal is handled by the L1XERC20Gateway instance, and L1XERC20Gateway:inboundEscrowTransfer() is called for C token. Since C is an adapter, getXERC20() on C will be called, returning the same XERC20 token as the one used in token A. Since there are no additional checks, the attacker will receive a specified amount of valuable XERC20 tokens without providing any backing on L2.

    function _inboundEscrowTransfer(address _tokenOrAdapter, address _dest, uint256 _amount) internal virtual {
        address _token = _tokenOrAdapter;
    
        if (addressIsAdapter(_tokenOrAdapter)) {
            _token = IXERC20Adapter(_tokenOrAdapter).getXERC20();
        }
        XERC20(_token).mint(_dest, _amount);
    }
    

In summary, the attacker on L2 burns worthless D tokens to obtain a claim for C tokens on L1. However, on L1, a claim for C tokens is incorrectly accepted as a valid claim for valuable A tokens, allowing the attacker to steal A tokens.

Also, there exists an alternative exploit scenario

The attacker registers the C/D token pair on the L1XERC20Gateway gateway. The attacker on L1 mints worthless C tokens; it bridges them to L2 and obtains a claim to corresponding D tokens. However, since the D token contract is malicious and acts as an IXERC20Adapter, it returns XERC20 corresponding to a B token on the same gateway. This allows the attacker to mint valuable B tokens with worthless C tokens.

Remediations to consider

  • Do not allow permissionless token registrations on L1XERC20Gateway.
  • Prevent multiple tokens from representing the same underlying token.
M-1

L1XERC20Gateway.forceRegisterTokenToL2() may brick bridge or perform unexpected mints/burns

Topic
Trust model
Status
Wont Do
Impact
High
Likelihood
Low

L1XERC20Gateway inherits from the L1CustomGateway contract. The forceRegisterTokenToL2() function implementation provided by the L1CustomGateway is accessible only by the gateway owner.

This function updates l1ToL2Token mapping on L1 and L2 through the corresponding counterparty gateway. Since this function does not perform thorough input validations, the caller/owner is responsible for providing correct input.

If the caller/owner is compromised or performs an incorrect update using forceRegisterTokenToL2(), the bridge may stop working correctly. For example, it may set l2Token in the l1ToL2Token mapping to a non-expected contract address.

In addition, the L1XERC20Gateway contract owner, through the use of forceRegisterTokenToL2(), may register wrapper token (IXERC20Adapter) for existing tokens and create an alternative path for minting underlying tokens. This would allow the contract owner to perform the same actions as the attacker in the [C-1] issue.

Remediations to consider

  • Use multi-sig or governance to protect owner privileges, or
  • Disable forceRegisterTokenToL2() unless needed.
Response by Connext

Won't do. Multi-sig or governance are meant to be used as the owner of the L1XERC20Gateway.

L1XERC20Gateway is meant for being managed by a trusted entity, like the Arbitrum DAO, so the forceRegisterTokenToL2 is there as a safety net. If someone other than a token issuer miss-registered a token using an adapter, the forceRegisterTokenToL2 could be used for fixing the registration.

M-2

XERC20 mint and burn limits may affect bridge operation

Topic
DoS
Status
Wont Do
Impact
High
Likelihood
Low

For successful bridge operations, which include XERC20 tokens implemented by third parties, it is important that configured mint and burn limits for instances of custom gateways (L1XERC20Gateway, L1LockboxGateway, L2XERC20Gateway, L2LockboxGateway) are appropriate and flexible so as not to prevent normal bridge operation.

Compromised or malicious XERC20 owner may change limits through XERC20:setLimits() and cause all bridge operations in previously mentioned gateway contracts to fail whenever one of these two functions is part of their execution trace:

  • XERC20BaseGateway:_outboundEscrowTransfer()
  • XERC20BaseGateway:_inboundEscrowTransfer()

As a result, bridging from L1 to L2 and vice versa, as well as unwrapping from the XERC20 to the base token, may not work, resulting in DoS.

Remediations to consider:

  • Use multi-sig or governance to protect owner privileges.
Response by Connext

Won't do. Multi-sig or governance will be used as the owner of the L1XERC20Gateway.

L-1

Native token may get stuck in L1XERC20Adapter and L1ArbitrumEnabledXERC20 contracts

Topic
Input validation
Status
Impact
Low
Likelihood
Low

L1XERC20Adapter.registerTokenOnL2() and L1ArbitrumEnabledXERC20.registerTokenOnL2() functions accept native token in order to send to the gateway and router appropriate amounts defined by valueForGateway and valueForRouter, respectively:

// L1ArbitrumEnabled
function _registerTokenOnL2(
    address l2TokenAddress,
    uint256 maxSubmissionCostForGateway,
    uint256 maxSubmissionCostForRouter,
    uint256 maxGasForGateway,
    uint256 maxGasForRouter,
    uint256 gasPriceBid,
    uint256 valueForGateway,
    uint256 valueForRouter,
    address creditBackAddress
)
    internal
{
    IL1CustomGateway gateway = IL1CustomGateway(gatewayAddress);

    gateway.registerTokenToL2{ value: valueForGateway }(
        l2TokenAddress, maxGasForGateway, gasPriceBid, maxSubmissionCostForGateway, creditBackAddress
    );
    IL1GatewayRouter(gateway.router()).setGateway{ value: valueForRouter }(
        gatewayAddress, maxGasForRouter, gasPriceBid, maxSubmissionCostForRouter, creditBackAddress
    );
}

However, if the native token amount provided exceeds the sum of valueForGateway and valueForRouter amounts, it will remain stuck in the contract, as there is no way to retrieve the excess amount.

Remediations to Consider

  • Add validation to check that the function’s msg.value is correct

    function registerTokenOnL2(...)
    {
    +		require(msg.value == valueForGateway + valueForGateway, "WRONG_VALUE");
    
        _registerTokenOnL2(
            l2TokenAddress,
            maxSubmissionCostForGateway,
            maxSubmissionCostForRouter,
            maxGasForGateway,
            maxGasForRouter,
            gasPriceBid,
            valueForGateway,
            valueForGateway,
            creditBackAddress
        );
    }
    
Q-1

XERC20BaseAdapter contract doesn’t expose all XERC20’s view functions

Topic
Spec
Status
Addressed
Quality Impact
Low

L1XERC20Adapter contract specification in docs has the following:

Implements all the read-only functions of an XERC20 token which forwards the calls to the actual token related to this adapter, this would help a smoother integration with UI

However, the XERC20BaseAdapter contract does expose ALL of XERC20’s view functions. These are all the functions that the contract missed:

  • XERC20's:
    • mintingMaxLimitOf()
    • burningMaxLimitOf()
    • mintingCurrentLimitOf()
    • burningCurrentLimitOf()
    • FACTORY()
    • lockbox()
    • bridges()
  • ERC20Permit's:
    • nonces()
    • DOMAIN_SEPARATOR()
  • EIP712's:
    • eip712Domain()
  • Ownable's:
    • owner()

Remediations to consider

  • Add missing view functions, or
  • Update the docs and clarify what functions will be exposed.
Response by Connext

Addressed by updating the associated specification document.

Q-2

Incorrect documentation for finalizeInboundTransfer on L2

Topic
Spec
Status
Quality Impact
Low

Natspec for L2XERC20Gateway.finalizeInboundTransfer() indicates that if the corresponding L2 token is not yet deployed, it will deploy it. However, in the L2XERC20Gateway implementation, this scenario is handled by performing a withdrawal back to L1. Consider updating natspec to describe this specific behavior correctly.

   * @notice Mint on L2 upon L1 deposit.
=> * If token not yet deployed and symbol/name/decimal data is included, deploys StandardArbERC20
   * @dev Callable only by the L1ERC20Gateway.outboundTransfer method. For initial deployments of a token the L1
   * L1ERC20Gateway
=> * is expected to include the deployData. If not a L1 withdrawal is automatically triggered for the user
Q-3

Unnecessary Ownable in XERC20BaseAdapter

Topic
Spec
Status
Quality Impact
Low

XERC20BaseAdapter contract features Ownable OZ functionality. However, this functionality is never used.

Consider removing it unless needed for specific use case.

I-1

Fee on transfer and rebase tokens are not supported

Topic
Token standard
Status
Addressed
Impact
Informational

Fee on transfer and rebase tokens are not supported by the current implementation. Therefore, relevant actors should not register tokens with these incompatible behavior patterns.

Response by Connext

Wont do. This solution is not meant to support those kind of tokens. We may address this in future update.

I-2

L1XERC20Gateway mint/burn limits should be set after successful XERC20 token registration

Topic
Use cases
Impact
Informational

In the L1XERC20Gateway, if mint/burn limits are set for a particular XERC20 token before it is registered, a malicious attacker may front-run token registration by registering an IXERC20Adapter wrapper for the same token. With successful wrapper token registration, the attacker may then mint the XERC20 token up to the approved limit for L1XERC20Gateway.

Due to the above, the XERC20 registration procedure and the order of steps performed should be carefully considered.

Response by Connext

We have updated documentation to indicate additional security requirements.

Disclaimer

Macro makes no warranties, either express, implied, statutory, or otherwise, with respect to the services or deliverables provided in this report, and Macro specifically disclaims all implied warranties of merchantability, fitness for a particular purpose, noninfringement and those arising from a course of dealing, usage or trade with respect thereto, and all such warranties are hereby excluded to the fullest extent permitted by law.

Macro will not be liable for any lost profits, business, contracts, revenue, goodwill, production, anticipated savings, loss of data, or costs of procurement of substitute goods or services or for any claim or demand by any other party. In no event will Macro be liable for consequential, incidental, special, indirect, or exemplary damages arising out of this agreement or any work statement, however caused and (to the fullest extent permitted by law) under any theory of liability (including negligence), even if Macro has been advised of the possibility of such damages.

The scope of this report and review is limited to a review of only the code presented by the Connext team and only the source code Macro notes as being within the scope of Macro’s review within this report. This report does not include an audit of the deployment scripts used to deploy the Solidity contracts in the repository corresponding to this audit. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. In this report you may through hypertext or other computer links, gain access to websites operated by persons other than Macro. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such websites’ owners. You agree that Macro is not responsible for the content or operation of such websites, and that Macro shall have no liability to your or any other person or entity for the use of third party websites. Macro assumes no responsibility for the use of third party software and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.