From ee83c3f41f47fd3197ea456f795dad1785e454a8 Mon Sep 17 00:00:00 2001 From: Ronny Haryanto Date: Tue, 19 May 2026 23:39:09 +1000 Subject: [PATCH] DecodeTxErr: prevent potential DoS caused by a malformed tx or block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A malformed tx or block with a varint count claiming more entries than the remaining buffer could possibly encode (e.g. a 0xff prefix declaring max-uint64 inputs) would make readVinVout/readBlock/readMerkleBranch loop billions of times against an exhausted stream, growing slices of zero-valued records until OOM. Recover() in consumers can't catch this — the goroutine never panics, it just never returns. Add tight lower-bound size constants (minTxInBytes=41, minTxOutBytes=9, minTxBytes=60, minHashBytes=32) and reject any varint count that would exceed remaining-bytes / min-per-element. Applied at every varint-driven loop in block.go: tx_count, merkle-branch hashes, vin_count, vout_count, and the witness stack count + per-item length. TestDecodeTxErr_MaliciousVinCount covers both the huge-varint case and the truncated-count case under a 500ms deadline, so a regression fails the test instead of hanging CI. --- block.go | 41 +++++++++++++++++++++++++++++++++++++++++ block_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/block.go b/block.go index 31f59c0..64703e3 100644 --- a/block.go +++ b/block.go @@ -11,8 +11,27 @@ const ( CoinbaseVOut = 0xffffffff MaxScriptSize = 10_000 // MAX_SCRIPT_SIZE from Dogecoin Core (script.h) MaxVarIntSize = 0x02000000 // MAX_SIZE from Dogecoin Core (serialize.h) + + // Minimum on-wire byte lengths for the elements of a transaction. These + // tight lower bounds let varint-driven loops bail out before allocating + // huge slices or spinning uint64-max iterations against an exhausted + // stream — defending against the "varint claims more entries than the + // buffer could possibly encode" class of malformed input. + minTxInBytes = 32 + 4 + 1 + 4 // prevout + vout + script_len(0) + sequence = 41 + minTxOutBytes = 8 + 1 // value + script_len(0) = 9 + minTxBytes = 4 + 1 + minTxInBytes + 1 + minTxOutBytes + 4 // version + vin_count + minimal vin + vout_count + minimal vout + locktime = 60 + minHashBytes = 32 // a merkle-branch hash ) +// remaining returns the number of unread bytes in the stream, clamped to 0 +// when the stream has already overflowed (s.pos > s.len). +func streamRemaining(s *Stream) uint64 { + if s.pos >= s.len { + return 0 + } + return s.len - s.pos +} + type HashID []byte func (id HashID) ToHex() string { @@ -112,6 +131,9 @@ func readBlock(s *Stream, calcHash bool) (b Block, err error) { b.AuxPoW = mtx } numTx := s.VarUint() + if numTx > streamRemaining(s)/minTxBytes { + return b, fmt.Errorf("invalid block: declared tx count %d exceeds remaining buffer (%d bytes)", numTx, streamRemaining(s)) + } for i := uint64(0); i < numTx; i++ { tx, err := readTx(s, calcHash) if err != nil { @@ -156,6 +178,12 @@ func readMerkleTx(s *Stream, calcHash bool) (*MerkleTx, error) { func readMerkleBranch(s *Stream) (b MerkleBranch) { numHash := s.VarUint() + if numHash > streamRemaining(s)/minHashBytes { + // Mark the stream invalid so callers see truncation instead of + // silently producing a short hash list. + s.pos = s.len + 1 + return + } for i := uint64(0); i < numHash; i++ { b.Hash = append(b.Hash, s.Bytes(32)) } @@ -232,11 +260,18 @@ func readTx(s *Stream, calcHash bool) (tx BlockTx, err error) { flags ^= 1 // Core toggles the low bit. for i := uint64(0); i < tx_in; i++ { numStackItems := s.VarUint() + // Each stack item is at least 1 byte (its own length varint). + if numStackItems > streamRemaining(s) { + return tx, fmt.Errorf("invalid transaction: witness stack count %d for vin %v exceeds remaining buffer (%d bytes)", numStackItems, i, streamRemaining(s)) + } for k := uint64(0); k < numStackItems; k++ { itemLen := s.VarUint() if itemLen > MaxVarIntSize { return tx, fmt.Errorf("invalid transaction: witness data too large: %v for vin %v stack item %v", itemLen, i, k) } + if itemLen > streamRemaining(s) { + return tx, fmt.Errorf("invalid transaction: witness item length %d for vin %v stack item %v exceeds remaining buffer (%d bytes)", itemLen, i, k, streamRemaining(s)) + } itemData := s.Bytes(itemLen) tx.VIn[i].Witness = append(tx.VIn[i].Witness, itemData) } @@ -275,6 +310,9 @@ func readTx(s *Stream, calcHash bool) (tx BlockTx, err error) { } func readVinVout(s *Stream, tx_in uint64) (VIn []BlockTxIn, VOut []BlockTxOut, err error) { + if tx_in > streamRemaining(s)/minTxInBytes { + return nil, nil, fmt.Errorf("invalid transaction: declared input count %d exceeds remaining buffer (%d bytes)", tx_in, streamRemaining(s)) + } for i := uint64(0); i < tx_in; i++ { vin, err := readTxIn(s) if err != nil { @@ -283,6 +321,9 @@ func readVinVout(s *Stream, tx_in uint64) (VIn []BlockTxIn, VOut []BlockTxOut, e VIn = append(VIn, vin) } tx_out := s.VarUint() + if tx_out > streamRemaining(s)/minTxOutBytes { + return nil, nil, fmt.Errorf("invalid transaction: declared output count %d exceeds remaining buffer (%d bytes)", tx_out, streamRemaining(s)) + } for i := uint64(0); i < tx_out; i++ { vout, err := readTxOut(s) if err != nil { diff --git a/block_test.go b/block_test.go index 3874269..0fba980 100644 --- a/block_test.go +++ b/block_test.go @@ -4,6 +4,7 @@ import ( "bytes" "reflect" "testing" + "time" ) func TestBlock(t *testing.T) { @@ -121,3 +122,35 @@ func TestSegWitTx(t *testing.T) { t.Errorf("TestSegWitTx: wrong transaction hash: %v vs %v", tx.TxID.ToHex(), segWitTxHash) } } + +// TestDecodeTxErr_MaliciousVinCount is a regression test for the previously +// unbounded "varint claims more entries than the buffer could possibly +// encode" class of input. Before bounds checking, the readVinVout loop ran +// uint64-max times against an exhausted stream, consuming CPU/memory until +// process exit. The decoder must now return an error promptly instead. +func TestDecodeTxErr_MaliciousVinCount(t *testing.T) { + cases := map[string][]byte{ + // Version + 0xff prefix = 8-byte varint claiming max-uint64 vins. + "huge_vin_count": {0x01, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + // Version + vin_count varint = 100, no vin body — the bounds check + // should also reject this since 100*41 > remaining. + "truncated_vins": {0x01, 0x00, 0x00, 0x00, 0x64}, + } + for name, raw := range cases { + t.Run(name, func(t *testing.T) { + done := make(chan error, 1) + go func() { + _, err := DecodeTxErr(raw, false) + done <- err + }() + select { + case err := <-done: + if err == nil { + t.Errorf("expected error for input %x, got nil", raw) + } + case <-time.After(500 * time.Millisecond): + t.Fatalf("DecodeTxErr did not return within 500ms for input %x — bounds check missing", raw) + } + }) + } +}