Operator can manipulate critical batch parameters, requires access control improvement.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L2688-L2714 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3686 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L2688-L2714 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3678-L3693

Vulnerability details

Impact

Overpowered operator permission can arbitrarily set the base fee and batch parameters like timestamp with unsafeOverrideBatch(). This seems dangerous.

Proof of Concept

The unsafeOverrideBatch() method that allows the operator to arbitrarily set the base fee and batch parameters is https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L2688-L2714

<!-- @if BOOTLOADER_TYPE=='playground_batch' -->

function unsafeOverrideBatch(newTimestamp, newBatchNumber, baseFee) {
  // logic to override batch params
}

<!-- @endif -->

This method is conditionally compiled only for "playground" batches, not production "proved" batches. But allowing the operator to manipulate critical batch parameters like this in any context seems dangerous.

At minimum, unsafeOverrideBatch() should be removed entirely to prevent misuse. Even better would be to add permissions restricting it to authorized accounts if override functionality is truly needed.

The problems are:

• No access control on a powerful method.
• Allows operator to manipulate critical data like base fee.
• Exists even in playground mode which could lead to mistakes enabling it in production.

This specific line of code that allows the operator to arbitrarily override the base fee and batch parameters.

Line 358:

unsafeOverrideBatch(NEW_BATCH_TIMESTAMP, NEW_BATCH_NUMBER, EXPECTED_BASE_FEE)

The unsafeOverrideBatch() method is defined on https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L2688-L2714

<!-- @if BOOTLOADER_TYPE=='playground_batch' -->

function unsafeOverrideBatch(newTimestamp, newBatchNumber, baseFee) {
  // logic to override batch params  
}

<!-- @endif -->

This method is called on https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3678-L3693, inside the logic that sets up the initial batch parameters:

<!-- @if BOOTLOADER_TYPE=='playground_batch' -->

switch SHOULD_SET_NEW_BATCH  
case 0:     
  unsafeOverrideBatch(NEW_BATCH_TIMESTAMP, NEW_BATCH_NUMBER, EXPECTED_BASE_FEE)
default:
  setNewBatch(PREV_BATCH_HASH, NEW_BATCH_TIMESTAMP, NEW_BATCH_NUMBER, EXPECTED_BASE_FEE)

<!-- @endif -->  

The problem is that in "playground" mode, the operator can call unsafeOverrideBatch to set the base fee and timestamps to arbitrary values. This could be abused to manipulate the state or grief users.

The batch parameters should be set via the setNewBatch() method which validates them properly. unsafeOverrideBatch() allows bypassing these validations, and should be removed or restricted to avoid potential abuse by the operator.

Consider this solid proof that the operator can actually manipulate the batch parameters in an uncontrolled way via the unsafeOverrideBatch() method.

Here is clear evidence this is possible:

1. unsafeOverrideBatch() is defined on https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L2688-L2714 with no access control:

<!-- @if BOOTLOADER_TYPE=='playground_batch' --> 

function unsafeOverrideBatch(newTimestamp, newBatchNumber, baseFee) {
  // logic to override  
}

<!-- @endif -->

2. On https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3686, it is called with arbitrary user-provided values:

unsafeOverrideBatch(NEW_BATCH_TIMESTAMP, NEW_BATCH_NUMBER, EXPECTED_BASE_FEE)

3. NEW_BATCH_TIMESTAMP, NEW_BATCH_NUMBER and EXPECTED_BASE_FEE are defined as: 3638, 3643, 3656

let NEW_BATCH_TIMESTAMP := mload(64) 
let NEW_BATCH_NUMBER := mload(96)
let EXPECTED_BASE_FEE := mload(192) 

4. These values come from unvalidated operator-supplied data loaded into memory.

5. Therefore, the operator can supply any values they want into memory, and use unsafeOverrideBatch() to set the batch params to those unvalidated values.

This confirms the operator has uncontrolled manipulation of critical batch parameters like base fee when playground mode is enabled. Removing or restricting access to unsafeOverrideBatch() is necessary to prevent potential abuse.

Tools Used

Manual

Recommended Mitigation Steps

Removing or permissioning this method will prevent potential abuse by the operator.

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as low quality report

[miladpiri]

Only for playground_batch.

[c4-sponsor]

miladpiri (sponsor) disputed

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid