code-423n4 / 2024-03-zksync-findings

1 stars 1 forks source link

If an L1 Receiver gets or is blacklisted by an L1 Token, funds will be permanently locked in L1SharedBridge #63

Closed c4-bot-8 closed 4 months ago

c4-bot-8 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/zksync/contracts/bridge/L2SharedBridge.sol#L123 https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/bridge/L1SharedBridge.sol#L452

Vulnerability details

Impact

The inability to update the _l1Receiver address upon blacklisting could lead to permanent locking of funds within the L1SharedBridge.

Proof of Concept

In L2SharedBridge.sol::withdraw(), users can initiate a withdrawal by burning funds on the contract and sending a message to L1, where tokens would be unlocked. Note that funds will be sent to the _l1Receiver, and there is no way to change this address after calling withdraw() since the funds are burned on L2 and the message has been sent to L1. Link to code

    function withdraw(address _l1Receiver, address _l2Token, uint256 _amount) external override {
        require(_amount > 0, "Amount cannot be zero");

        IL2StandardToken(_l2Token).bridgeBurn(msg.sender, _amount);

        address l1Token = l1TokenAddress[_l2Token];
        require(l1Token != address(0), "yh");

        bytes memory message = _getL1WithdrawMessage(_l1Receiver, l1Token, _amount);
        L2ContractHelper.sendMessageToL1(message);

        emit WithdrawalInitiated(msg.sender, _l1Receiver, _l2Token, _amount);
    }

Then, the user can unlock the funds by calling finalizeWithdrawal on L1SharedBridge, which sends the tokens to the _l1Receiver specified on L2: Link to code

            // Withdraw funds
            IERC20(l1Token).safeTransfer(l1Receiver, amount);
        }

Given that some widely used tokens like USDC, Tether, etc., have blacklisting mechanisms, in the case when _l1Receiver gets or is blacklisted in an L1 token like USDC, Tether, etc., funds will be permanently locked within L1SharedBridge.

Tools Used

Foundry VSCode

Recommended Mitigation Steps

Empower users with the ability to update the _l1Receiver address in case of blacklisting or other exceptional circumstances.

One of the possible solutions is to implement an ERC20 token, let's call it ZToken, which is pegged to the L1 token. In this proposed approach, during the withdrawal process, the _l1Receiver would receive ZToken instead of the L1 token directly. Subsequently, the _l1Receiver can redeem the ZToken for the corresponding L1 token. This mechanism provides flexibility in scenarios where the redemption fails due to the address being blacklisted. If the redemption transaction fails because the _l1Receiver is blacklisted on the L1 token, they can transfer the ZToken to another address that is not blacklisted in L1 token.

Assessed type

ERC20

c4-sponsor commented 5 months ago

saxenism (sponsor) disputed

saxenism commented 5 months ago

We are aware of the risks and therefore we have a permissioned system for accepting tokens into the bridge and any non-standard implementation of ERC20 tokens won't be allowed. We consider this a non-issue.

c4-sponsor commented 5 months ago

saxenism (sponsor) acknowledged

alex-ppg commented 4 months ago

The Warden describes how a blacklist will affect bridging functionality when the intended recipient is blacklisted. Such behavior is considered acceptable and coupled with the permissioned token list of the bridge is considered OOS.

c4-judge commented 4 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope