security(rollup): _commitBatch does not validate lastBlockNumber > parent (empty block span → node DoS)

[curryxbo]

Summary

Rollup.sol::_commitBatch accepts an unconstrained lastBlockNumber from calldata. There is no on-chain check that the committed batch's lastBlockNumber is greater than the parent batch's lastBlockNumber (i.e. that the batch contains at least one block / blockCount >= 1).

Because the BLS signature path is still a stub (_getBLSMsgHash returns bytes32(0)), this field is not signature-bound either, so a single active staker (byzantine or merely buggy) can commit a malformed lastBlockNumber on-chain.

This is the root cause of the node-side batch-parse DoS fixed in #994 / #995. Those PRs harden the derivation node so a malformed blockCount becomes a clean parse error instead of a panic — but the malformed batch is still accepted on L1, which leaves every verifier node permanently stalled on that batch (it cannot be skipped without diverging from the L1-canonical chain). The only complete fix is to reject the malformed value at the source, on-chain.

Impact

• A single active staker can commit a batch with lastBlockNumber == parent.lastBlockNumber (empty span, blockCount == 0).
• Pre-fix(derivation): reject invalid blockCount to prevent batch-parse DoS #995 derivation nodes in layer1 verify mode panic (no recover() in the derivation goroutine) → process crash / crash-loop network-wide.
• Post-fix(derivation): reject invalid blockCount to prevent batch-parse DoS #995 nodes return a parse error and stall derivation/verification permanently on that batch.
• The field is not consensus-authenticated (BLS stub), so this needs no collusion.

Affected code

• contracts/contracts/l1/rollup/Rollup.sol::_commitBatch (L888) — stores lastBlockNumber into the data hash with no validation.
• contracts/contracts/l1/rollup/Rollup.sol::_commitBatchWithBatchData — the shared path for commitBatch / commitState / commitBatchWithProof.
• Related: _getBLSMsgHash (L788) is a return bytes32(0) stub, so the field is not signature-bound (tracked separately).

Proposed fix (this issue)

In _commitBatchWithBatchData, after loading the parent header and before computing the data hash, enforce a non-empty, monotonic block span when the parent header carries a lastBlockNumber (V1+):

if (BatchHeaderCodecV0.getVersion(_batchPtr) >= 1) {
    require(
        batchDataInput.lastBlockNumber > BatchHeaderCodecV1.getLastBlockNumber(_batchPtr),
        "empty block span"
    );
}

This mirrors the node's blockCount = lastBlockNumber - parentLastBlockNumber derivation. The V0->V1 transition (parent version 0) carries no lastBlockNumber and is a historical one-time event, so it is not guarded here.

Out of scope (follow-up)

• Implementing _getBLSMsgHash so batch fields (incl. lastBlockNumber) are bound to a real BLS signature over the sequencer set (_getValidSequencerSet is also a TODO). This is the deeper, systemic fix and is tracked separately.
• An upper-bound span cap (MAX_BLOCKS_PER_BATCH) as additional defense-in-depth; the node side (fix(derivation): reject invalid blockCount to prevent batch-parse DoS #995) already bounds allocation by payload length.

Notes

This is an L1 contract change and requires the normal upgrade path (audit + timelock/governance). #995 (node-side mitigation) ships independently and faster; this issue is the source-level closure.