c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L239

Vulnerability details

Impact

The inefficiency in the synchronization of the upgrade mechanism with batch commitment poses a challenge to the zkSync Era protocol. The problem arises when a scheduled L2 upgrade is executed while there are batches pending commitment on L1.

Proof of Concept

When a scheduled L2 upgrade is ready for execution, it proceeds without consideration of any batches that may be in the process of being committed on L1. In simpler terms, imagine there are several commitBatches transactions in the Ethereum mempool, and before they get mined, the scheduled L2 upgrade transaction is executed. https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/zksync/ValidatorTimelock.sol#L77

This upgrade transaction alters the state of the system by setting s.l2SystemContractsUpgradeTxHash to a non-zero value, and it may also make changes to s.l2BootloaderBytecodeHash or s.l2DefaultAccountBytecodeHash. https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/upgrades/DefaultUpgrade.sol#L31 https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/upgrades/DefaultUpgrade.sol#L34

However, once the commitBatches transactions are eventually mined, they identify that s.l2SystemContractsUpgradeTxHash is non-zero and expect the first batch to include this upgrade transaction. But, in reality, these batches were generated before the scheduled L2 upgrade transaction was executed. https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L189 https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L239

When processing the L2Logs related to this batch, it becomes evident that the batch doesn't contain this L2 upgrade transaction. As a result, only seven logs are found. However, since _expectedSystemContractUpgradeTxHash is non-zero, it anticipates eight logs. This discrepancy leads to a revert at Line 169 in the code. https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L43 https://github.com/code-423n4/2023-10-zksync/blob/main/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L169

The core issue here is the lack of synchronization between the upgrade mechanism and the commitment of batches.

Tools Used

Recommended Mitigation Steps

A best practice would involve associating the scheduled Layer 2 (L2) upgrade transaction with the number of the last committed batch with some margin. This approach helps prevent interference with batches awaiting commitment in the Ethereum mempool.