Lines of code
<https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/libraries/SystemContractHelper.sol#L48&gt;

Vulnerability details

Impact

Context

To initiate a withdrawal from L2 to L1, a user can call L2EthToken.withdraw method, then funds will be available to calim on L1 via finalizeEthWithdrawal method of MailboxFacet.

function withdraw(address _l1Receiver) external payable override

<https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/L2EthToken.sol#L80&gt;

The mechanism behind this is that thewithdraw method sends a constructed withdraw message to L1 via L1Messenger’s sendToL1 method, then the user can use the message as a proof in order to claim the funds on L1.

        // Send the L2 log, a user could use it as proof of the withdrawal
        bytes memory message = _getL1WithdrawMessage(_l1Receiver, amount);
        L1_MESSENGER_CONTRACT.sendToL1(message);

<https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/L2EthToken.sol#L90-L91&gt;

Possible loss of funds

If the withdrawal transaction succeeds, the user should be able to claim his/her funds on L1 always. However, there is a case where the withdrawal transaction succeeds even though the message wasn’t sent to L1. This could happen if there is not enough gas during message sending.
As a result, the user loses funds since the ether are burnt in L2 but can not be claimed on L1 due to the failure of sending the message to L1.

Note: the withdraw method should be called via MsgValueSimulator since msg.value is set by the simulator.

Proof of Concept

The execution sequence as follows:

flowchart TD;
   	subgraph L2EthToken
   withdraw-->L2EthToken._getL1WithdrawMessage(_getL1WithdrawMessage)
   end
   	subgraph L1Messenger
sendToL1;		
   	end
   	subgraph SystemContractHelper
toL1;		
   	end
   	L2EthToken-->L1Messenger
   	L1Messenger-->SystemContractHelper

Let’s have a look at SystemContractHelper.toL1 method:

    function toL1(bool _isService, bytes32 _key, bytes32 _value) internal {
        address callAddr = TO_L1_CALL_ADDRESS;
        assembly {
            // Ensuring that the type is bool
            _isService := and(_isService, 1)
            // This success is always 0, but the method always succeeds
            // (except for the cases when there is not enough gas)
            let success := call(_isService, callAddr, _key, _value, 0xFFFF, 0, 0)
        }
    }

<https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/libraries/SystemContractHelper.sol#L48&gt;

There is call to TO_L1_CALL_ADDRESS address, please note that this is just a simulation to zkSync VM-specific v1.3.0 opcodes. For more info, check this:
<https://github.com/code-423n4/2023-03-zksync/blob/main/docs/VM-specific_v1.3.0_opcodes_simulation.pdf&gt;

As noticed, the method should always succeed except when there is not enough gas. In this case, the method could possibly fail silently leading to a successfull withdraw transaciton without having the message sent to L1.
Since the gas limit is the minimum of operatorTrustedErgsLimit and gasLimit provided by the user, it is possible the issue occurs if both:

1. The gasLimit provided by the user was picked up because it was lower than operatorTrustedErgsLimit.
   <https://github.com/code-423n4/2023-03-zksync/blob/main/bootloader/bootloader.yul#L1128-L1129&gt;
2. The gasLimit couldn’t cover L1 messaging cost but was enough to make the withdraw transaction succeeds.

For more details about to_l1_message_opcode, have a look at eraSyncVM relevant code:

• <https://github.com/matter-labs/era-sync_vm/blob/main/src/vm/vm_cycle/opcode_execution/log.rs#L16&gt;
• <https://github.com/matter-labs/era-sync_vm/blob/main/src/vm/vm_cycle/opcode_execution/log.rs#L243&gt;

Tools Used

Manual analysis

Recommended Mitigation Steps

One possibility is to change the behaviour of to_l1_message_opcode to return a failure in case there wasn’t enough gas and then validate it. But this fix would be on a VM level and might not be preferred.
Another possibility is to add a check in L1 method to assert there is a minimum gas to cover the messaging as it’s critical especially when used for bridging funds from L2 to L1.

---

The text was updated successfully, but these errors were encountered:

All reactions