Skip to content

test(l2): pin DivisionByZero in calculate_l1_fee_gas on validation gap#6538

Draft
avilagaston9 wants to merge 1 commit intomainfrom
security/l1-fee-gas-divzero
Draft

test(l2): pin DivisionByZero in calculate_l1_fee_gas on validation gap#6538
avilagaston9 wants to merge 1 commit intomainfrom
security/l1-fee-gas-divzero

Conversation

@avilagaston9
Copy link
Copy Markdown
Contributor

Motivation

calculate_l1_fee_gas (called from reserve_l1_gas during
prepare_execution) computes
l1_fee_gas = l1_fee.checked_div(vm.env.gas_price) and surfaces a
zero divisor as InternalError::DivisionByZero. None of the upstream
validators reject gas_price == 0 in this configuration:

  • default_hook::validate_sufficient_max_fee_per_gas only rejects
    max_fee_per_gas < base_fee_per_gas. With base_fee_per_gas = 0,
    any max_fee_per_gas — including 0 — passes.
  • validate_sufficient_max_fee_per_gas_l2 early-returns when
    operator_fee_config is None.

So a transaction with all of base_fee_per_gas, max_fee_per_gas,
max_priority_fee_per_gas at zero — a configuration that can occur
on quiet L2s or in early blocks — passes validation and divides by
zero in calculate_l1_fee_gas as soon as the L1 fee config is
enabled. The error surfaces as
VMError::Internal(InternalError::DivisionByZero), the
"invariant violated" path. A user-controlled input must never reach
that path.

This is a sibling of #6537 (operator-fee priority underflow): same
class of bug — incomplete L2-hook validation lets user input drive
the math into an internal-error fallback — but a different code path
and a different trigger (zero gas_price hits reserve_l1_gas
before pay_coinbase_l2, so the iteration-1 underflow doesn't
shadow it). Filed separately so each path has its own reproducer.

Description

Adds a regression test
zero_gas_price_with_l1_fee_config_divides_by_zero_in_reserve in
test/tests/levm/l2_gas_reservation_tests.rs that builds an
EIP-1559 transaction with all gas params at zero, an environment
where base_fee_per_gas and gas_price are both zero, and an L2
fee config that has L1 fee enabled and no operator fee (the latter
isolates this finding from #6537). The test pins the current (buggy)
outcome — the VM returns
VMError::Internal(InternalError::DivisionByZero). The assertion is
written so that any change in error handling — for example,
tightening validation to require gas_price > 0 when an L1 fee
config is set, or making calculate_l1_fee_gas short-circuit to a
TxValidationError (e.g. IntrinsicGasTooLow /
InsufficientMaxFeePerGas) — breaks this test, prompting an
explicit update of the expected error.

The fix is intentionally not in this PR. Two reasonable shapes:

  1. Tighten validation to require gas_price > 0 (or
    >= base_fee + operator_fee + 1, paralleling test(l2): pin operator-fee priority-fee underflow on validation gap #6537) when an L1
    fee config is set, returning a TxValidationError.
  2. Have calculate_l1_fee_gas short-circuit to Ok(0) when
    l1_fee == 0 && gas_price == 0, and return a TxValidationError
    when l1_fee > 0 && gas_price == 0 (the user is trying to skip
    paying for L1 DA).

Reproduction

cargo test -p ethrex-test --test ethrex_tests \
  zero_gas_price_with_l1_fee_config_divides_by_zero_in_reserve

The test passes on main (the bug is real and reachable).

Impact

A user-submitted L2 transaction with gas_price == 0 and an L1 fee
config active reaches the internal-error path during
prepare_execution. Whether the consequence is "tx silently dropped
from the block" or "block production aborts" depends on the caller's
handling of VMError::Internal, but in either case the validator
should reject the tx upfront — internal errors are not the right
escape hatch for malformed user input. Trigger requires
base_fee_per_gas == 0, which is reachable on quiet/dev L2s.

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

…ion.

`calculate_l1_fee_gas` (called from `reserve_l1_gas` during
`prepare_execution`) computes
`l1_fee_gas = l1_fee.checked_div(vm.env.gas_price)` and surfaces a
zero divisor as `InternalError::DivisionByZero`. None of the upstream
validators reject `gas_price == 0` in this configuration:
`default_hook::validate_sufficient_max_fee_per_gas` only rejects
`max_fee_per_gas < base_fee_per_gas`, so with `base_fee_per_gas = 0`
any `max_fee_per_gas` (including 0) passes; and
`validate_sufficient_max_fee_per_gas_l2` early-returns when
`operator_fee_config` is `None`. So a transaction with all of
`base_fee_per_gas`, `max_fee_per_gas`, `max_priority_fee_per_gas`
set to zero — a configuration that occurs on quiet L2s or in early
blocks — gets past validation and divides by zero in
`calculate_l1_fee_gas` as soon as the L1 fee config is enabled.
The error surfaces as `VMError::Internal(InternalError::DivisionByZero)`,
the "invariant violated" path; user-controlled inputs must never
reach it.

The new test
`zero_gas_price_with_l1_fee_config_divides_by_zero_in_reserve`
in `test/tests/levm/l2_gas_reservation_tests.rs` builds an EIP-1559
transaction with all gas params at zero, an environment where
`base_fee_per_gas` and `gas_price` are both zero, and an L2 fee
config that has L1 fee enabled but no operator fee (the latter so
the related operator-fee underflow path doesn't shadow this one).
The test asserts the resulting
`VMError::Internal(InternalError::DivisionByZero)` and is written
so that any change in error handling — for example, replacing the
internal-error path with a proper `TxValidationError` once the
validator is tightened — fails the assertion, prompting an explicit
update.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L2 Rollup client

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant