diff --git a/node/derivation/batch_info.go b/node/derivation/batch_info.go index fa8f6bd15..344e1cb9b 100644 --- a/node/derivation/batch_info.go +++ b/node/derivation/batch_info.go @@ -177,15 +177,26 @@ func (bi *BatchInfo) ParseBatch(batch geth.RPCRollupBatch) error { txsData = batchBytes } else { // Block contexts are at the head of the decompressed stream, - // immediately followed by the tx payload bytes. - bcLen := blockCount * 60 - if uint64(len(batchBytes)) < bcLen { - return fmt.Errorf("decompressed batch too short for block contexts: have %d, need %d", len(batchBytes), bcLen) + // immediately followed by the tx payload bytes. Bound blockCount by + // the payload length using division so blockCount*60 cannot overflow + // uint64 and bypass the length guard (a wrapped-small bcLen would + // otherwise drive a huge make([]*BlockContext) below). + if blockCount > uint64(len(batchBytes))/60 { + return fmt.Errorf("decompressed batch too short for block contexts: have %d, need %d blocks", len(batchBytes), blockCount) } + bcLen := blockCount * 60 rawBlockContexts = batchBytes[:bcLen] txsData = batchBytes[bcLen:] } + // A batch must contain at least one block; zero-block batches are not + // supported. blockCount == 0 (e.g. a batch whose lastBlockNumber equals the + // parent's, or a zero block-count prefix) would yield empty blockContexts + // and a nil header downstream during derivation. Reject it here. + if blockCount == 0 { + return fmt.Errorf("invalid batch: zero block count") + } + data, err := commonbatch.DecodeTxsFromBytes(txsData) if err != nil { return err diff --git a/node/derivation/batch_info_test.go b/node/derivation/batch_info_test.go index 416aafbdc..e3f1852ce 100644 --- a/node/derivation/batch_info_test.go +++ b/node/derivation/batch_info_test.go @@ -2,6 +2,7 @@ package derivation import ( "crypto/rand" + "math" "math/big" "testing" @@ -224,3 +225,50 @@ func TestParseBatchMultiBlobConcatDecompressInvariant(t *testing.T) { "reversed-blob decompression unexpectedly matched payload") } } + +// TestParseBatchBlockCountBounds covers the blockCount guards hardened in +// issue #994, both of which would otherwise panic and crash layer1-verify +// nodes. A zero count (lastBlockNumber == parent) yielded empty blockContexts +// and a nil-header deref during derivation; zero-block batches are unsupported +// and must be rejected. A huge count let blockCount*60 overflow uint64, bypass +// the length guard, and drive an out-of-range make([]*BlockContext); it must +// likewise be rejected. ParseBatch must return an error, not panic. +func TestParseBatchBlockCountBounds(t *testing.T) { + const ( + parentIndex = 7 + startBlock = 1_000 + ) + // Minimal valid payload (one block context + empty tx-stream terminator) + // so ParseBatch decompresses cleanly and reaches blockCount validation. + payload := append(buildBlockContexts(startBlock, 1), 0x00) + compressed, err := zstd.CompressBatchBytes(payload) + require.NoError(t, err) + blobs := splitCompressedIntoBlobs(t, compressed) + + parentHeader := buildV1ParentHeader(parentIndex, startBlock) // parent.LastBlockNumber == startBlock-1 + + t.Run("zero block count is rejected", func(t *testing.T) { + batch := geth.RPCRollupBatch{ + Version: 1, + ParentBatchHeader: parentHeader, + LastBlockNumber: startBlock - 1, // == parent => blockCount 0 + Sidecar: eth.BlobTxSidecar{Blobs: blobs}, + } + var bi BatchInfo + require.ErrorContains(t, bi.ParseBatch(batch), "zero block count") + }) + + t.Run("overflowing block count is rejected", func(t *testing.T) { + batch := geth.RPCRollupBatch{ + Version: 1, + ParentBatchHeader: parentHeader, + // blockCount = LastBlockNumber - (startBlock-1) chosen so that + // blockCount*60 wraps uint64; the division-based guard rejects it + // before the multiply. + LastBlockNumber: (startBlock - 1) + (math.MaxUint64/60 + 2), + Sidecar: eth.BlobTxSidecar{Blobs: blobs}, + } + var bi BatchInfo + require.Error(t, bi.ParseBatch(batch)) + }) +}