Validator may refuse to process the cross-chain transaction.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L152
https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L208
https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L221
https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L336

Vulnerability details

Impact

Currently the project trust model is that the governor and the validator to not act maliciously.

But if the validator refuse to process the cross-chain transaction, the user's fund is locked infinitely in the L1ERC20Bridge.sol and in L1ETHBridge.sol

Proof of Concept

In the current implementation, when user starts a cross-chain transfer, the user needs to call deposit and the cross-chain is put into a queue.

The validator needs to pop from the queue to process the job in first-in-first-out order.

However, if the validator just refuse to process the cross-chain transaction by calling the function that has the modifier onlyValidator in the Executor.sol. The transaction in the queue is stucked.

function commitBlocks(StoredBlockInfo memory _lastCommittedBlockData, CommitBlockInfo[] calldata _newBlocksData)
	external
	override
	nonReentrant
	onlyValidator
{

and

    function executeBlocks(StoredBlockInfo[] calldata _blocksData) external nonReentrant onlyValidator {

and

    function proveBlocks(
        StoredBlockInfo calldata _prevBlock,
        StoredBlockInfo[] calldata _committedBlocks,
        ProofInput calldata _proof
    ) external nonReentrant onlyValidator {

Tools Used

VS Code

Recommended Mitigation Steps

If the validator turns rogue, there is no cost for validator but the cost from the project and the cost from user is great. It is time confusing for governor and project to set up new validator infra.

I think a way to mitigate this to make sure the validator does the job is let them stake tokens and if they does not process transaction,

The protocol punish the validator by slashing their tokens.

[miladpiri]

As mentioned in the overview of zkSync in C4 contest:

L1 -> L2 communication is implemented as requesting an L2 transaction on L1 and executing it on L2. This means a user can call the function on L1 contract to save the data about the transaction in some queue. Later on, a validator can process such transactions on L2 and mark them as processed on the L1 priority queue. Currently, it is used only for sending information from L1 to L2 or implementing a multi-layer protocol, but it is planned to use a priority queue for the censor-resistance mechanism.

So, invalid issue!

[c4-sponsor]

miladpiri marked the issue as sponsor disputed

[GalloDaSballo]

Per the sponsor comment, with acknowledgement on the readme, also considering the possibility of performing a FullExit:
https://etherscan.io/address/0x8e972b354e6933275513c355ee14d44a832ad2d9#code#F1#L326

Am closing as invalid

[c4-judge]

GalloDaSballo marked the issue as nullified