Merge pull request #2558 from starius/trailing

multi: reject trailing characters when parsing inputs

blockchain/chainio.go

@@ -1277,8 +1277,7 @@ func (b *BlockChain) initChainState() error {
 		if err != nil {
 			return err
 		}
-		var block wire.MsgBlock
-		err = block.Deserialize(bytes.NewReader(blockBytes))
+		block, err := btcutil.NewBlockFromBytes(blockBytes)
 		if err != nil {
 			return err
 		}
@@ -1304,8 +1303,8 @@ func (b *BlockChain) initChainState() error {
 
 		// Initialize the state related to the best block.
 		blockSize := uint64(len(blockBytes))
-		blockWeight := uint64(GetBlockWeight(btcutil.NewBlock(&block)))
-		numTxns := uint64(len(block.Transactions))
+		blockWeight := uint64(GetBlockWeight(block))
+		numTxns := uint64(len(block.MsgBlock().Transactions))
 		b.stateSnapshot = newBestState(tip, blockSize, blockWeight,
 			numTxns, state.totalTxns, CalcPastMedianTime(tip))
 

blockchain/chainio_test.go

@@ -9,9 +9,12 @@ import (
 	"errors"
 	"math/big"
 	"reflect"
+	"strings"
 	"testing"
 
+	"github.com/btcsuite/btcd/btcutil/v2"
 	"github.com/btcsuite/btcd/database"
+	"github.com/btcsuite/btcd/txscript/v2"
 	"github.com/btcsuite/btcd/wire/v2"
 )
 
@@ -37,6 +40,52 @@ func TestErrNotInMainChain(t *testing.T) {
 	}
 }
 
+// TestInitChainStateRejectsTrailingBestBlockBytes ensures startup rejects a
+// stored best block whose bytes contain a valid block plus trailing data.
+func TestInitChainStateRejectsTrailingBestBlockBytes(t *testing.T) {
+	chain, params, teardown := utxoCacheTestChain(
+		"TestInitChainStateRejectsTrailingBestBlockBytes")
+	defer teardown()
+
+	tip := btcutil.NewBlock(params.GenesisBlock)
+	tip.SetHeight(0)
+
+	block, _, err := newBlock(chain, tip, nil)
+	if err != nil {
+		t.Fatalf("failed to build block: %v", err)
+	}
+
+	var serialized bytes.Buffer
+	err = block.MsgBlock().Serialize(&serialized)
+	if err != nil {
+		t.Fatalf("failed to serialize block: %v", err)
+	}
+
+	trailingBytes := append([]byte(nil), serialized.Bytes()...)
+	trailingBytes = append(trailingBytes, 0x00)
+	trailingBlock := btcutil.NewBlockFromBlockAndBytes(
+		block.MsgBlock(), trailingBytes,
+	)
+
+	_, _, err = chain.ProcessBlock(trailingBlock, BFNone)
+	if err != nil {
+		t.Fatalf("failed to process block: %v", err)
+	}
+
+	_, err = New(&Config{
+		DB:          chain.db,
+		ChainParams: params,
+		TimeSource:  NewMedianTime(),
+		SigCache:    txscript.NewSigCache(1000),
+	})
+	if err == nil {
+		t.Fatal("expected trailing best block bytes to fail startup")
+	}
+	if !strings.Contains(err.Error(), "trailing bytes") {
+		t.Fatalf("expected trailing byte error, got: %v", err)
+	}
+}
+
 // TestStxoSerialization ensures serializing and deserializing spent transaction
 // output entries works as expected.
 func TestStxoSerialization(t *testing.T) {

btcec/schnorr/musig2/sign.go

@@ -91,7 +91,7 @@ func (p *PartialSignature) Decode(r io.Reader) error {
 
 	var sBytes [32]byte
 	if _, err := io.ReadFull(r, sBytes[:]); err != nil {
-		return nil
+		return err
 	}
 
 	overflows := p.S.SetBytes(&sBytes)

btcec/schnorr/musig2/sign_test.go

@@ -311,6 +311,27 @@ func pSigsFromIndices(t *testing.T, sigs []string, indices []int) []*PartialSign
 	return pSigs
 }
 
+// TestPartialSignatureDecodeRejectsShortReads verifies that Decode rejects
+// inputs that do not contain a full scalar.
+func TestPartialSignatureDecodeRejectsShortReads(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string][]byte{
+		"empty":     nil,
+		"truncated": bytes.Repeat([]byte{0x01}, 31),
+	}
+
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var sig PartialSignature
+			err := sig.Decode(bytes.NewReader(testCase))
+			require.Error(t, err)
+		})
+	}
+}
+
 // TestMusig2SignCombine tests that we pass the musig2 sig combination tests.
 func TestMusig2SignCombine(t *testing.T) {
 	t.Parallel()

btcutil/block.go

@@ -253,6 +253,9 @@ func NewBlockFromBytes(serializedBlock []byte) (*Block, error) {
 	if err != nil {
 		return nil, err
 	}
+	if br.Len() > 0 {
+		return nil, fmt.Errorf("block has %d trailing bytes", br.Len())
+	}
 	b.serializedBlock = serializedBlock
 
 	// This initializes []btcutil.Tx to have the serialized raw

btcutil/block_test.go

@@ -200,6 +200,23 @@ func TestNewBlockFromBytes(t *testing.T) {
 	}
 }
 
+// TestNewBlockFromBytesRejectsTrailingData verifies that NewBlockFromBytes
+// rejects bytes after the serialized block.
+func TestNewBlockFromBytesRejectsTrailingData(t *testing.T) {
+	var block100000Buf bytes.Buffer
+	err := Block100000.Serialize(&block100000Buf)
+	if err != nil {
+		t.Errorf("Serialize: %v", err)
+	}
+
+	_, err = btcutil.NewBlockFromBytes(
+		append(block100000Buf.Bytes(), 0x00),
+	)
+	if err == nil {
+		t.Fatal("expected error for block with trailing data")
+	}
+}
+
 // TestNewBlockFromBlockAndBytes tests creation of a Block from a MsgBlock and
 // raw bytes.
 func TestNewBlockFromBlockAndBytes(t *testing.T) {

btcutil/bloom/filter_test.go

@@ -325,7 +325,7 @@ func TestFilterBloomMatch(t *testing.T) {
 		0x02, 0x00, 0x00, 0x00, 0x00, 0x19, 0x76, 0xa9, 0x14,
 		0xc1, 0x09, 0x32, 0x48, 0x3f, 0xec, 0x93, 0xed, 0x51,
 		0xf5, 0xfe, 0x95, 0xe7, 0x25, 0x59, 0xf2, 0xcc, 0x70,
-		0x43, 0xf9, 0x88, 0xac, 0x00, 0x00, 0x00, 0x00, 0x00}
+		0x43, 0xf9, 0x88, 0xac, 0x00, 0x00, 0x00, 0x00}
 
 	spendingTx, err := btcutil.NewTxFromBytes(spendingTxBytes)
 	if err != nil {

btcutil/tx.go

@@ -6,6 +6,7 @@ package btcutil
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 
 	"github.com/btcsuite/btcd/chainhash/v2"
@@ -173,7 +174,16 @@ func (t *Tx) setBytes(bytes []byte) {
 // serialized bytes.  See Tx.
 func NewTxFromBytes(serializedTx []byte) (*Tx, error) {
 	br := bytes.NewReader(serializedTx)
-	return NewTxFromReader(br)
+	tx, err := NewTxFromReader(br)
+	if err != nil {
+		return nil, err
+	}
+	if br.Len() > 0 {
+		return nil, fmt.Errorf("transaction has %d trailing bytes",
+			br.Len())
+	}
+
+	return tx, nil
 }
 
 // NewTxFromReader returns a new instance of a bitcoin transaction given a

btcutil/tx_test.go

@@ -76,6 +76,22 @@ func TestNewTxFromBytes(t *testing.T) {
 	}
 }
 
+// TestNewTxFromBytesRejectsTrailingData verifies that NewTxFromBytes rejects
+// bytes after the serialized transaction.
+func TestNewTxFromBytesRejectsTrailingData(t *testing.T) {
+	testTx := Block100000.Transactions[0]
+	var testTxBuf bytes.Buffer
+	err := testTx.Serialize(&testTxBuf)
+	if err != nil {
+		t.Errorf("Serialize: %v", err)
+	}
+
+	_, err = btcutil.NewTxFromBytes(append(testTxBuf.Bytes(), 0x00))
+	if err == nil {
+		t.Fatal("expected error for transaction with trailing data")
+	}
+}
+
 // TestTxErrors tests the error paths for the Tx API.
 func TestTxErrors(t *testing.T) {
 	// Serialize the test transaction.

go.mod

@@ -39,6 +39,11 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
+replace (
+	github.com/btcsuite/btcd/btcutil/v2 => ./btcutil
+	github.com/btcsuite/btcd/wire/v2 => ./wire
+)
+
 // The retract statements below fixes an accidental push of the tags of a btcd
 // fork.
 retract (

psbt/partial_input.go

@@ -90,9 +90,7 @@ func (pi *PInput) deserialize(r io.Reader) error {
 			if keyData != nil {
 				return ErrInvalidKeyData
 			}
-			tx := wire.NewMsgTx(2)
-
-			err := tx.Deserialize(bytes.NewReader(value))
+			tx, err := readTransaction(value, false)
 			if err != nil {
 				return err
 			}

psbt/psbt.go

@@ -188,12 +188,12 @@ func NewFromUnsignedTx(tx *wire.MsgTx) (*Packet, error) {
 // NOTE: To create a Packet from one's own data, rather than reading in a
 // serialization from a counterparty, one should use a psbt.New.
 func NewFromRawBytes(r io.Reader, b64 bool) (*Packet, error) {
-	// If the PSBT is encoded in bas64, then we'll create a new wrapper
-	// reader that'll allow us to incrementally decode the contents of the
-	// io.Reader.
 	if b64 {
-		based64EncodedReader := r
-		r = base64.NewDecoder(base64.StdEncoding, based64EncodedReader)
+		decoded, err := decodeBase64Strict(r)
+		if err != nil {
+			return nil, err
+		}
+		r = bytes.NewReader(decoded)
 	}
 
 	// The Packet struct does not store the fixed magic bytes, but they
@@ -225,11 +225,9 @@ func NewFromRawBytes(r io.Reader, b64 bool) (*Packet, error) {
 	if err != nil {
 		return nil, err
 	}
-	msgTx := wire.NewMsgTx(2)
-
 	// BIP-0174 states: "The transaction must be in the old serialization
 	// format (without witnesses)."
-	err = msgTx.DeserializeNoWitness(bytes.NewReader(value))
+	msgTx, err := readTransaction(value, true)
 	if err != nil {
 		return nil, err
 	}
@@ -326,9 +324,36 @@ func NewFromRawBytes(r io.Reader, b64 bool) (*Packet, error) {
 		return nil, err
 	}
 
+	if err := assertFullyConsumed(r); err != nil {
+		return nil, err
+	}
+
 	return &newPsbt, nil
 }
 
+// decodeBase64Strict decodes an RFC4648 base64 stream without permitting
+// whitespace and with '=' allowed only as final padding.
+func decodeBase64Strict(r io.Reader) ([]byte, error) {
+	encoded, err := io.ReadAll(r)
+	if err != nil {
+		return nil, err
+	}
+
+	// Go's strict base64 decoder still ignores CR/LF. Reject them before
+	// decoding so base64 PSBT parsing matches the RFC4648 alphabet exactly.
+	if bytes.ContainsAny(encoded, "\r\n") {
+		return nil, ErrInvalidPsbtFormat
+	}
+
+	decoded := make([]byte, base64.StdEncoding.DecodedLen(len(encoded)))
+	n, err := base64.StdEncoding.Strict().Decode(decoded, encoded)
+	if err != nil {
+		return nil, ErrInvalidPsbtFormat
+	}
+
+	return decoded[:n], nil
+}
+
 // Serialize creates a binary serialization of the referenced Packet struct
 // with lexicographical ordering (by key) of the subsections.
 func (p *Packet) Serialize(w io.Writer) error {

psbt/strict_tx_values_test.go

@@ -0,0 +1,268 @@
+package psbt
+
+import (
+	"bytes"
+	"encoding/base64"
+	"testing"
+
+	"github.com/btcsuite/btcd/wire/v2"
+	"github.com/stretchr/testify/require"
+)
+
+// strictnessTxPair returns a minimal unsigned transaction and the previous
+// transaction provided as its non-witness UTXO.
+func strictnessTxPair(t *testing.T) (*wire.MsgTx, *wire.MsgTx) {
+	t.Helper()
+
+	prevTx := wire.NewMsgTx(2)
+	prevTx.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: wire.OutPoint{Index: wire.MaxPrevOutIndex},
+		Sequence:         wire.MaxTxInSequenceNum,
+	})
+	prevTx.AddTxOut(&wire.TxOut{
+		Value:    12345,
+		PkScript: []byte{0x51},
+	})
+
+	unsignedTx := wire.NewMsgTx(2)
+	unsignedTx.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: wire.OutPoint{
+			Hash:  prevTx.TxHash(),
+			Index: 0,
+		},
+		Sequence: wire.MaxTxInSequenceNum,
+	})
+	unsignedTx.AddTxOut(&wire.TxOut{
+		Value:    1000,
+		PkScript: []byte{0x51},
+	})
+
+	return unsignedTx, prevTx
+}
+
+// serializeTxForStrictness serializes tx in the PSBT form required by the
+// field under test.
+func serializeTxForStrictness(t *testing.T, tx *wire.MsgTx,
+	noWitness bool) []byte {
+
+	t.Helper()
+
+	var buf bytes.Buffer
+	var err error
+	if noWitness {
+		err = tx.SerializeNoWitness(&buf)
+	} else {
+		err = tx.Serialize(&buf)
+	}
+	require.NoError(t, err)
+
+	return buf.Bytes()
+}
+
+// strictnessPSBT builds a minimal PSBT using the supplied transaction-valued
+// fields verbatim.
+func strictnessPSBT(t *testing.T, unsignedTx,
+	nonWitnessUtxo []byte) []byte {
+
+	t.Helper()
+
+	var buf bytes.Buffer
+	_, err := buf.Write(psbtMagic[:])
+	require.NoError(t, err)
+
+	require.NoError(t, serializeKVPairWithType(
+		&buf, byte(UnsignedTxType), nil, unsignedTx,
+	))
+	require.NoError(t, buf.WriteByte(0x00))
+
+	require.NoError(t, serializeKVPairWithType(
+		&buf, byte(NonWitnessUtxoType), nil, nonWitnessUtxo,
+	))
+	require.NoError(t, buf.WriteByte(0x00))
+	require.NoError(t, buf.WriteByte(0x00))
+
+	return buf.Bytes()
+}
+
+// serializeTxOutForStrictness serializes txOut in the PSBT form required by
+// the WitnessUtxo field.
+func serializeTxOutForStrictness(t *testing.T, txOut *wire.TxOut) []byte {
+	t.Helper()
+
+	var buf bytes.Buffer
+	require.NoError(t, wire.WriteTxOut(&buf, 0, 0, txOut))
+
+	return buf.Bytes()
+}
+
+// strictnessPSBTWithWitnessUtxo builds a minimal PSBT using the supplied
+// WitnessUtxo value verbatim.
+func strictnessPSBTWithWitnessUtxo(t *testing.T,
+	witnessUtxo []byte) []byte {
+
+	t.Helper()
+
+	unsignedTx, _ := strictnessTxPair(t)
+
+	var buf bytes.Buffer
+	_, err := buf.Write(psbtMagic[:])
+	require.NoError(t, err)
+
+	require.NoError(t, serializeKVPairWithType(
+		&buf, byte(UnsignedTxType), nil,
+		serializeTxForStrictness(t, unsignedTx, true),
+	))
+	require.NoError(t, buf.WriteByte(0x00))
+
+	require.NoError(t, serializeKVPairWithType(
+		&buf, byte(WitnessUtxoType), nil, witnessUtxo,
+	))
+	require.NoError(t, buf.WriteByte(0x00))
+	require.NoError(t, buf.WriteByte(0x00))
+
+	return buf.Bytes()
+}
+
+// TestRejectsTrailingDataInTransactionValues verifies that PSBT transaction
+// values must contain exactly one serialized transaction.
+func TestRejectsTrailingDataInTransactionValues(t *testing.T) {
+	unsignedTx, prevTx := strictnessTxPair(t)
+	unsignedTxBytes := serializeTxForStrictness(t, unsignedTx, true)
+	prevTxBytes := serializeTxForStrictness(t, prevTx, false)
+
+	testCases := []struct {
+		name           string
+		unsignedTx     []byte
+		nonWitnessUtxo []byte
+	}{{
+		name:           "global unsigned tx",
+		unsignedTx:     append(unsignedTxBytes, 0x00),
+		nonWitnessUtxo: prevTxBytes,
+	}, {
+		name:           "input non-witness utxo",
+		unsignedTx:     unsignedTxBytes,
+		nonWitnessUtxo: append(prevTxBytes, 0x00),
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := NewFromRawBytes(bytes.NewReader(
+				strictnessPSBT(
+					t, tc.unsignedTx, tc.nonWitnessUtxo,
+				),
+			), false)
+			require.ErrorIs(t, err, ErrInvalidPsbtFormat)
+		})
+	}
+}
+
+// TestRejectsTrailingDataAfterPacket verifies that extra bytes after a valid
+// PSBT packet are rejected.
+func TestRejectsTrailingDataAfterPacket(t *testing.T) {
+	unsignedTx, prevTx := strictnessTxPair(t)
+	rawPacket := strictnessPSBT(
+		t,
+		serializeTxForStrictness(t, unsignedTx, true),
+		serializeTxForStrictness(t, prevTx, false),
+	)
+
+	_, err := NewFromRawBytes(
+		bytes.NewReader(append(rawPacket, 0x00)), false,
+	)
+	require.ErrorIs(t, err, ErrInvalidPsbtFormat)
+}
+
+// TestRejectsNonCanonicalBase64Packet verifies that base64 PSBT input rejects
+// whitespace, bad padding, and extra decoded packet bytes.
+func TestRejectsNonCanonicalBase64Packet(t *testing.T) {
+	unsignedTx, prevTx := strictnessTxPair(t)
+	rawPacket := strictnessPSBT(
+		t,
+		serializeTxForStrictness(t, unsignedTx, true),
+		serializeTxForStrictness(t, prevTx, false),
+	)
+	encoded := base64.StdEncoding.EncodeToString(rawPacket)
+	insert := func(idx int, s string) string {
+		return encoded[:idx] + s + encoded[idx:]
+	}
+
+	testCases := []struct {
+		name    string
+		encoded string
+	}{{
+		name:    "trailing LF",
+		encoded: encoded + "\n",
+	}, {
+		name:    "trailing CRLF",
+		encoded: encoded + "\r\n",
+	}, {
+		name:    "LF between groups",
+		encoded: insert(4, "\n"),
+	}, {
+		name:    "LF inside group",
+		encoded: insert(5, "\n"),
+	}, {
+		name:    "space between groups",
+		encoded: insert(4, " "),
+	}, {
+		name:    "tab inside group",
+		encoded: insert(5, "\t"),
+	}, {
+		name:    "padding in middle",
+		encoded: insert(len(encoded)-4, "="),
+	}, {
+		name: "extra decoded bytes",
+		encoded: base64.StdEncoding.EncodeToString(
+			append(append([]byte{}, rawPacket...), 0x00),
+		),
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := NewFromRawBytes(
+				bytes.NewReader([]byte(tc.encoded)), true,
+			)
+			require.ErrorIs(t, err, ErrInvalidPsbtFormat)
+		})
+	}
+}
+
+// TestParsesWitnessUtxoTxOutStrictly verifies that WitnessUtxo values are
+// parsed as exact transaction outputs.
+func TestParsesWitnessUtxoTxOutStrictly(t *testing.T) {
+	pkScript := bytes.Repeat([]byte{0x51}, 253)
+	txOutBytes := serializeTxOutForStrictness(t, &wire.TxOut{
+		Value:    1234,
+		PkScript: pkScript,
+	})
+
+	packet, err := NewFromRawBytes(bytes.NewReader(
+		strictnessPSBTWithWitnessUtxo(t, txOutBytes),
+	), false)
+	require.NoError(t, err)
+	require.Equal(t, pkScript, packet.Inputs[0].WitnessUtxo.PkScript)
+
+	malformedTxOut := append(append([]byte{}, txOutBytes...), 0x00)
+	_, err = NewFromRawBytes(bytes.NewReader(
+		strictnessPSBTWithWitnessUtxo(t, malformedTxOut),
+	), false)
+	require.ErrorIs(t, err, ErrInvalidPsbtFormat)
+}
+
+// TestParsesWitnessUtxoTxOutCompactSizeScriptLength verifies that WitnessUtxo
+// scripts with multi-byte CompactSize lengths are parsed without folding the
+// length bytes into the script.
+func TestParsesWitnessUtxoTxOutCompactSizeScriptLength(t *testing.T) {
+	pkScript := bytes.Repeat([]byte{0x51}, 300)
+	txOutBytes := serializeTxOutForStrictness(t, &wire.TxOut{
+		Value:    1234,
+		PkScript: pkScript,
+	})
+	require.Equal(t, byte(0xfd), txOutBytes[8])
+
+	packet, err := NewFromRawBytes(bytes.NewReader(
+		strictnessPSBTWithWitnessUtxo(t, txOutBytes),
+	), false)
+	require.NoError(t, err)
+	require.Equal(t, pkScript, packet.Inputs[0].WitnessUtxo.PkScript)
+}

psbt/utils.go

@@ -6,7 +6,6 @@ package psbt
 
 import (
 	"bytes"
-	"encoding/binary"
 	"errors"
 	"fmt"
 	"io"
@@ -278,17 +277,69 @@ func getKey(r io.Reader) (int, []byte, error) {
 	return int(keyType), keyData, nil
 }
 
-// readTxOut is a limited version of wire.ReadTxOut, because the latter is not
-// exported.
-func readTxOut(txout []byte) (*wire.TxOut, error) {
-	if len(txout) < 10 {
-		return nil, ErrInvalidPsbtFormat
+// assertFullyConsumed returns ErrInvalidPsbtFormat if r still has bytes
+// available after parsing.
+func assertFullyConsumed(r io.Reader) error {
+	if lr, ok := r.(interface{ Len() int }); ok {
+		if lr.Len() > 0 {
+			return ErrInvalidPsbtFormat
+		}
+
+		return nil
 	}
 
-	valueSer := binary.LittleEndian.Uint64(txout[:8])
-	scriptPubKey := txout[9:]
+	var trailing [1]byte
+	_, err := io.ReadFull(r, trailing[:])
+	switch {
+	case err == nil:
+		return ErrInvalidPsbtFormat
 
-	return wire.NewTxOut(int64(valueSer), scriptPubKey), nil
+	case errors.Is(err, io.EOF):
+		return nil
+
+	default:
+		return err
+	}
+}
+
+// readTxOut parses a transaction output value and requires the full value to
+// be consumed.
+func readTxOut(txout []byte) (*wire.TxOut, error) {
+	txOut := &wire.TxOut{}
+	reader := bytes.NewReader(txout)
+
+	if err := wire.ReadTxOut(reader, 0, 0, txOut); err != nil {
+		return nil, err
+	}
+	if err := assertFullyConsumed(reader); err != nil {
+		return nil, err
+	}
+
+	return txOut, nil
+}
+
+// readTransaction parses a transaction value and requires the full value to be
+// consumed. PSBT transaction-valued fields contain exactly one network
+// serialized transaction, not a transaction prefix with arbitrary trailing data.
+func readTransaction(txBytes []byte, noWitness bool) (*wire.MsgTx, error) {
+	tx := wire.NewMsgTx(2)
+	reader := bytes.NewReader(txBytes)
+
+	var err error
+	if noWitness {
+		err = tx.DeserializeNoWitness(reader)
+	} else {
+		err = tx.Deserialize(reader)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	if err := assertFullyConsumed(reader); err != nil {
+		return nil, err
+	}
+
+	return tx, nil
 }
 
 // SumUtxoInputValues tries to extract the sum of all inputs specified in the

rpcserver.go

@@ -791,22 +791,22 @@ func handleDecodeRawTransaction(s *rpcServer, cmd interface{}, closeChan <-chan
 	if err != nil {
 		return nil, rpcDecodeHexError(hexStr)
 	}
-	var mtx wire.MsgTx
-	err = mtx.Deserialize(bytes.NewReader(serializedTx))
+	tx, err := btcutil.NewTxFromBytes(serializedTx)
 	if err != nil {
 		return nil, &btcjson.RPCError{
 			Code:    btcjson.ErrRPCDeserialization,
 			Message: "TX decode failed: " + err.Error(),
 		}
 	}
+	mtx := tx.MsgTx()
 
 	// Create and return the result.
 	txReply := btcjson.TxRawDecodeResult{
 		Txid:     mtx.TxHash().String(),
 		Version:  mtx.Version,
 		Locktime: mtx.LockTime,
-		Vin:      createVinList(&mtx),
-		Vout:     createVoutList(&mtx, s.cfg.ChainParams, nil),
+		Vin:      createVinList(mtx),
+		Vout:     createVoutList(mtx, s.cfg.ChainParams, nil),
 	}
 	return txReply, nil
 }
@@ -2141,14 +2141,13 @@ func handleGetBlockTemplateProposal(s *rpcServer, request *btcjson.TemplateReque
 				"hexadecimal string (not %q)", hexData),
 		}
 	}
-	var msgBlock wire.MsgBlock
-	if err := msgBlock.Deserialize(bytes.NewReader(dataBytes)); err != nil {
+	block, err := btcutil.NewBlockFromBytes(dataBytes)
+	if err != nil {
 		return nil, &btcjson.RPCError{
 			Code:    btcjson.ErrRPCDeserialization,
 			Message: "Block decode failed: " + err.Error(),
 		}
 	}
-	block := btcutil.NewBlock(&msgBlock)
 
 	// Ensure the block is building from the expected previous block.
 	expectedPrevHash := s.cfg.Chain.BestSnapshot().Hash
@@ -3441,8 +3440,7 @@ func handleSendRawTransaction(s *rpcServer, cmd interface{}, closeChan <-chan st
 	if err != nil {
 		return nil, rpcDecodeHexError(hexStr)
 	}
-	var msgTx wire.MsgTx
-	err = msgTx.Deserialize(bytes.NewReader(serializedTx))
+	tx, err := btcutil.NewTxFromBytes(serializedTx)
 	if err != nil {
 		return nil, &btcjson.RPCError{
 			Code:    btcjson.ErrRPCDeserialization,
@@ -3451,7 +3449,6 @@ func handleSendRawTransaction(s *rpcServer, cmd interface{}, closeChan <-chan st
 	}
 
 	// Use 0 for the tag to represent local node.
-	tx := btcutil.NewTx(&msgTx)
 	acceptedTxs, err := s.cfg.TxMemPool.ProcessTransaction(tx, false, false, 0)
 	if err != nil {
 		// When the error is a rule error, it means the transaction was

rpcserver_test.go

@@ -1,15 +1,18 @@
 package main
 
 import (
+	"bytes"
 	"encoding/hex"
 	"errors"
 	"testing"
 
 	"github.com/btcsuite/btcd/btcjson"
 	"github.com/btcsuite/btcd/btcutil/v2"
+	"github.com/btcsuite/btcd/chaincfg/v2"
 	"github.com/btcsuite/btcd/chainhash/v2"
 	"github.com/btcsuite/btcd/mempool"
 	"github.com/btcsuite/btcd/wire/v2"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -67,6 +70,119 @@ func TestHandleTestMempoolAcceptFailDecode(t *testing.T) {
 	}
 }
 
+// TestHandleTestMempoolAcceptRejectsTrailingBytes ensures testmempoolaccept
+// rejects byte strings that contain a valid transaction plus trailing data.
+func TestHandleTestMempoolAcceptRejectsTrailingBytes(t *testing.T) {
+	t.Parallel()
+
+	defer func() {
+		recovered := recover()
+		require.Nil(t, recovered, "handler reached mempool")
+	}()
+
+	cmd := btcjson.NewTestMempoolAcceptCmd([]string{txHex1 + "00"}, 0)
+	result, err := handleTestMempoolAccept(
+		&rpcServer{}, cmd, make(chan struct{}),
+	)
+
+	requireRPCErrorCode(t, err, btcjson.ErrRPCDeserialization)
+	require.Nil(t, result)
+}
+
+// requireRPCErrorCode asserts that the error is an RPC error with the expected
+// error code.
+func requireRPCErrorCode(t *testing.T, err error, code btcjson.RPCErrorCode) {
+	t.Helper()
+
+	require.Error(t, err)
+	rpcErr, ok := err.(*btcjson.RPCError)
+	require.True(t, ok)
+	require.Equal(t, code, rpcErr.Code)
+}
+
+// blockHexWithTrailingByte serializes a valid block and appends one extra byte.
+func blockHexWithTrailingByte(t *testing.T) string {
+	t.Helper()
+
+	var block bytes.Buffer
+	err := chaincfg.MainNetParams.GenesisBlock.Serialize(&block)
+	require.NoError(t, err)
+
+	return hex.EncodeToString(append(block.Bytes(), 0x00))
+}
+
+// TestHandleSendRawTransactionRejectsTrailingBytes ensures sendrawtransaction
+// rejects byte strings that contain a valid transaction plus trailing data.
+func TestHandleSendRawTransactionRejectsTrailingBytes(t *testing.T) {
+	t.Parallel()
+
+	mm := &mempool.MockTxMempool{}
+	mm.On(
+		"ProcessTransaction", mock.Anything, false, false, mempool.Tag(0),
+	).Return(nil, errors.New("mempool should not be reached")).Maybe()
+
+	s := &rpcServer{cfg: rpcserverConfig{
+		TxMemPool: mm,
+	}}
+	cmd := btcjson.NewSendRawTransactionCmd(txHex1+"00", nil)
+
+	result, err := handleSendRawTransaction(s, cmd, make(chan struct{}))
+	requireRPCErrorCode(t, err, btcjson.ErrRPCDeserialization)
+	require.Nil(t, result)
+}
+
+// TestHandleDecodeRawTransactionRejectsTrailingBytes ensures
+// decoderawtransaction rejects byte strings that contain a valid transaction
+// plus trailing data.
+func TestHandleDecodeRawTransactionRejectsTrailingBytes(t *testing.T) {
+	t.Parallel()
+
+	cmd := btcjson.NewDecodeRawTransactionCmd(txHex1 + "00")
+	result, err := handleDecodeRawTransaction(
+		&rpcServer{}, cmd, make(chan struct{}),
+	)
+
+	requireRPCErrorCode(t, err, btcjson.ErrRPCDeserialization)
+	require.Nil(t, result)
+}
+
+// TestHandleGetBlockTemplateProposalRejectsTrailingBytes ensures proposal mode
+// rejects byte strings that contain a valid block plus trailing data.
+func TestHandleGetBlockTemplateProposalRejectsTrailingBytes(t *testing.T) {
+	t.Parallel()
+
+	defer func() {
+		recovered := recover()
+		require.Nil(t, recovered, "handler reached chain state")
+	}()
+
+	request := &btcjson.TemplateRequest{
+		Mode: "proposal",
+		Data: blockHexWithTrailingByte(t),
+	}
+
+	result, err := handleGetBlockTemplateProposal(&rpcServer{}, request)
+	requireRPCErrorCode(t, err, btcjson.ErrRPCDeserialization)
+	require.Nil(t, result)
+}
+
+// TestHandleSubmitBlockRejectsTrailingBytes ensures submitblock rejects byte
+// strings that contain a valid block plus trailing data.
+func TestHandleSubmitBlockRejectsTrailingBytes(t *testing.T) {
+	t.Parallel()
+
+	defer func() {
+		recovered := recover()
+		require.Nil(t, recovered, "handler reached sync manager")
+	}()
+
+	cmd := btcjson.NewSubmitBlockCmd(blockHexWithTrailingByte(t), nil)
+	result, err := handleSubmitBlock(&rpcServer{}, cmd, make(chan struct{}))
+
+	requireRPCErrorCode(t, err, btcjson.ErrRPCDeserialization)
+	require.Nil(t, result)
+}
+
 var (
 	// TODO(yy): make a `btctest` package and move these testing txns there
 	// so they be used in other tests.

wire/message.go

@@ -570,6 +570,12 @@ func ReadV2MessageN(plaintext []byte, pver uint32, enc MessageEncoding) (
 		return nil, nil, err
 	}
 
+	if buf.Len() > 0 {
+		str := fmt.Sprintf("message payload has %d extra bytes "+
+			"after decode", buf.Len())
+		return nil, nil, messageError("ReadV2MessageN", str)
+	}
+
 	return msg, plaintext, nil
 }
 

wire/message_test.go

@@ -347,7 +347,6 @@ func TestReadMessageWireErrors(t *testing.T) {
 			ErrUnknownMessage,
 			24,
 		},
-
 	}
 
 	t.Logf("Running %d tests", len(tests))
@@ -428,6 +427,28 @@ func TestReadMessageTrailingBytes(t *testing.T) {
 	}
 }
 
+// TestReadV2MessageTrailingBytes verifies that a v2 message with unconsumed
+// trailing bytes after BtcDecode is rejected with a MessageError.
+func TestReadV2MessageTrailingBytes(t *testing.T) {
+	payload := []byte{
+		v2Messages[CmdInv],
+		0x00, // zero inventory vectors
+		0xaa, // trailing data not consumed by MsgInv.BtcDecode
+	}
+
+	_, _, err := ReadV2MessageN(
+		payload, ProtocolVersion, BaseEncoding,
+	)
+	if err == nil {
+		t.Fatal("expected error for v2 message with trailing bytes")
+	}
+
+	var msgErr *MessageError
+	if !errors.As(err, &msgErr) {
+		t.Fatalf("expected MessageError, got: %T (%v)", err, err)
+	}
+}
+
 // TestWriteMessageWireErrors performs negative tests against wire encoding from
 // concrete messages to confirm error paths work correctly.
 func TestWriteMessageWireErrors(t *testing.T) {