search

code-423n4 / 2023-10-zksync-findings

4 stars 0 forks source link

Possible law issues with Circle and Tether #258

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/zksync/contracts/bridge/L2StandardERC20.sol#L46 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/zksync/contracts/bridge/L2ERC20Bridge.sol#L76-L80

Vulnerability details

As this is not a full "bug-code issue", I will not put the typical "Impact-POC-RMS" pattern. The underlying issue is that the L2ERC20Bridge is deploying and linking tokens without taking into account their logic, that is, blacklisting, fee-on-transfer, rebasing... this example is the most dangerous IMO, as it affects a huge amount of money that is in production right now.

Intro

Circle and Tether do back the two most popular stablecoins in the world, namely USDC and USDT. One of the main characteristics that differentiates them from other tokens is that they have a blacklisting functionality, that is, they can block hackers and bad actors from using "their" tokens (I guess it is done to prevent law issues in which Circle or Tether "back" fraudulent or illegal activity, as those stablecoins are overcollateralized with fiat currency in their hands so, in practice, hackers and bad actors would be using Circle's or Tether's money).

Because of this, their counterparts on many L2 do have such a functionality to avoid bad actors from "laundering" their fraudulent money by bridging it to other layers (see on Polygon or Arbitrum, for example). However, zkSync Era does not follow this so MatterLabs may incur in law issues with Circle and Tether because of that, as "their product" has been "modified" (knowingly or unknowingly) to "avoid corporate decisions" and is being used in the wild "as is".

What

Let's review the bridging of ERC20 tokens from a high level:

  1. ANY user can deposit ANY ERC20 compatible token by calling the L1ERC20Bridge, function deposit, which locks the tokens on the bridge and sends a request to the operator with the next token metadata retrieved from the token contract in Ethereum:

L1ERC20Bridge, lines 240 to 245

    function _getERC20Getters(address _token) internal view returns (bytes memory data) {
        (, bytes memory data1) = _token.staticcall(abi.encodeCall(IERC20Metadata.name, ()));
        (, bytes memory data2) = _token.staticcall(abi.encodeCall(IERC20Metadata.symbol, ()));
        (, bytes memory data3) = _token.staticcall(abi.encodeCall(IERC20Metadata.decimals, ()));
        data = abi.encode(data1, data2, data3);
    }
  1. The request is sent and the bootloader executes the transaction by calling finalizeDeposit on the L2ERC20Bridge contract. If the given token is not registered as known, it is deployed as an L2StandardERC20 token, without taking into account the actual bytecode in Ethereum (as it is not passed in the metadata above):

L2ERC20Bridge, lines 63 to 88

    function finalizeDeposit(
        address _l1Sender,
        address _l2Receiver,
        address _l1Token,
        uint256 _amount,
        bytes calldata _data
    ) external payable override {

        ...

        address expectedL2Token = l2TokenAddress(_l1Token);
        address currentL1Token = l1TokenAddress[expectedL2Token];

        if (currentL1Token == address(0)) { // not registered as know

            address deployedToken = _deployL2Token(_l1Token, _data); // deploys a beacon proxy and the mimic token
            require(deployedToken == expectedL2Token, "mt");
            l1TokenAddress[expectedL2Token] = _l1Token; // link its address with its L1 counterpart (unmodifiable)

        }

        ...
    }

  1. If we see the code of the L2standardERC20 token, it is just a dummy clone of its Ethereum counterpart, that is, it has the above metadata and the transfer/mint/burn/etc functionality of a typical OpenZeppelin ERC20Upgradeable token (not gonna paste the full code here, go read the code by yourself). That means no blacklisting functionality, nor rebasing, nor fee-on-transfer... (that's another issue).

  2. As the bridge stores the relation between the l1Token with the newly deployed token inside the l1TokenAddress mapping and there is no way to update it, even if Circle or Tether deploys their own tokens in zkSync Era with the correct functionalities, the link between their L1 counterpart is already done so the deployed bridge won't be able to "re-link" the Ethereum USDC/USDT address with the correct one. This is a big issue as, right now, there are around 109M USDC and 9.5M USDT locked in the L1ERC20Bridge (see here, which can be freely used by anyone on zkSync Era, as their counterparts on zkSync Era are simple L2StandardERC20 tokens, so the impacts below.

Impacts

Hacker's POV

Circle/Tether's POV

I don't have time to read the legal terms or any US laws they are subject to. However, "using their product" like it's used right now (see the locked amounts and the transactions being made), which have been "modified" (knowingly or unknowingly) to "avoid corporate decisions" about their "own products" makes the scenario of law issues against MatterLabs real (although remote AF).

Moreover, I haven't read much of the case against TornadoCash by OFAC but, as things are going RN, if hackers ever launder their stolen tokens by bridging them to zkSync Era (that is, avoid sanctions and be able to safeguard and use their bounty without losing value), it may be possible to suffer from law issues which could harm the future of MatterLabs.

Assessed type

Other

bytes032 commented 1 year ago

suitable for analysis report

c4-pre-sort commented 1 year ago

bytes032 marked the issue as low quality report

c4-judge commented 12 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)