Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions node/derivation/batch_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions node/derivation/batch_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package derivation

import (
"crypto/rand"
"math"
"math/big"
"testing"

Expand Down Expand Up @@ -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))
})
}
Loading