multi: relax trailing byte handling for DB blocks and PSBT readers

blockchain/chainio.go

@@ -1277,7 +1277,7 @@ func (b *BlockChain) initChainState() error {
 		if err != nil {
 			return err
 		}
-		block, err := btcutil.NewBlockFromBytes(blockBytes)
+		block, err := DBBlockFromBytes(blockBytes, state.hash)
 		if err != nil {
 			return err
 		}
@@ -1301,8 +1301,15 @@ func (b *BlockChain) initChainState() error {
 			}
 		}
 
-		// Initialize the state related to the best block.
-		blockSize := uint64(len(blockBytes))
+		// Initialize the state related to the best block.  The block
+		// bytes are re-derived from the block itself so any trailing
+		// bytes ignored during deserialization are excluded from the
+		// recorded size.
+		serializedBlock, err := block.Bytes()
+		if err != nil {
+			return err
+		}
+		blockSize := uint64(len(serializedBlock))
 		blockWeight := uint64(GetBlockWeight(block))
 		numTxns := uint64(len(block.MsgBlock().Transactions))
 		b.stateSnapshot = newBestState(tip, blockSize, blockWeight,
@@ -1367,6 +1374,35 @@ func dbFetchHeaderByHeight(dbTx database.Tx, height int32) (*wire.BlockHeader, e
 	return dbFetchHeaderByHash(dbTx, hash)
 }
 
+// DBBlockFromBytes deserializes a block fetched from the local database,
+// tolerating trailing bytes rather than rejecting them outright.  Databases
+// written by older btcd versions may have persisted blocks with trailing
+// bytes, and failing here would make such blocks permanently unreadable.
+// Instead, any trailing bytes are logged, ignored, and excluded from the
+// serialization cached in the returned block.
+//
+// This lenient parsing is only appropriate for blocks read back from the
+// node's own database.  Blocks from external sources (p2p, RPC) should be
+// parsed with the strict btcutil.NewBlockFromBytes instead.
+func DBBlockFromBytes(blockBytes []byte, hash chainhash.Hash) (*btcutil.Block,
+	error) {
+
+	blockReader := bytes.NewReader(blockBytes)
+	var msgBlock wire.MsgBlock
+	if err := msgBlock.Deserialize(blockReader); err != nil {
+		return nil, err
+	}
+	if trailing := blockReader.Len(); trailing > 0 {
+		log.Warnf("Block %v has %d trailing bytes in the database; "+
+			"ignoring them", hash, trailing)
+		blockBytes = blockBytes[:len(blockBytes)-trailing]
+	}
+
+	// Cache the exact serialization on the block so downstream consumers
+	// of the raw bytes never observe the trailing bytes.
+	return btcutil.NewBlockFromBlockAndBytes(&msgBlock, blockBytes), nil
+}
+
 // dbFetchBlockByNode uses an existing database transaction to retrieve the
 // raw block for the provided node, deserialize it, and return a btcutil.Block
 // with the height set.
@@ -1378,7 +1414,7 @@ func dbFetchBlockByNode(dbTx database.Tx, node *blockNode) (*btcutil.Block, erro
 	}
 
 	// Create the encapsulated block and set the height appropriately.
-	block, err := btcutil.NewBlockFromBytes(blockBytes)
+	block, err := DBBlockFromBytes(blockBytes, node.hash)
 	if err != nil {
 		return nil, err
 	}

blockchain/chainio_test.go

@@ -9,10 +9,10 @@ import (
 	"errors"
 	"math/big"
 	"reflect"
-	"strings"
 	"testing"
 
 	"github.com/btcsuite/btcd/btcutil/v2"
+	"github.com/btcsuite/btcd/chaincfg/v2"
 	"github.com/btcsuite/btcd/database"
 	"github.com/btcsuite/btcd/txscript/v2"
 	"github.com/btcsuite/btcd/wire/v2"
@@ -40,11 +40,13 @@ func TestErrNotInMainChain(t *testing.T) {
 	}
 }
 
-// TestInitChainStateRejectsTrailingBestBlockBytes ensures startup rejects a
+// TestInitChainStateToleratesTrailingBestBlockBytes ensures startup loads a
 // stored best block whose bytes contain a valid block plus trailing data.
-func TestInitChainStateRejectsTrailingBestBlockBytes(t *testing.T) {
+// Databases written by older btcd versions may contain such blocks, so
+// rejecting them would prevent the node from ever starting.
+func TestInitChainStateToleratesTrailingBestBlockBytes(t *testing.T) {
 	chain, params, teardown := utxoCacheTestChain(
-		"TestInitChainStateRejectsTrailingBestBlockBytes")
+		"TestInitChainStateToleratesTrailingBestBlockBytes")
 	defer teardown()
 
 	tip := btcutil.NewBlock(params.GenesisBlock)
@@ -72,17 +74,83 @@ func TestInitChainStateRejectsTrailingBestBlockBytes(t *testing.T) {
 		t.Fatalf("failed to process block: %v", err)
 	}
 
-	_, err = New(&Config{
+	restarted, 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 err != nil {
+		t.Fatalf("expected trailing best block bytes to be "+
+			"tolerated at startup, got: %v", err)
+	}
+
+	// The best state must reflect the stored block, with the trailing
+	// byte excluded from the recorded block size.
+	snapshot := restarted.BestSnapshot()
+	if snapshot.Hash != *block.Hash() {
+		t.Fatalf("unexpected best block hash - got %v, want %v",
+			snapshot.Hash, block.Hash())
 	}
-	if !strings.Contains(err.Error(), "trailing bytes") {
-		t.Fatalf("expected trailing byte error, got: %v", err)
+	wantSize := uint64(len(serialized.Bytes()))
+	if snapshot.BlockSize != wantSize {
+		t.Fatalf("unexpected best block size - got %d, want %d",
+			snapshot.BlockSize, wantSize)
+	}
+}
+
+// TestDBBlockFromBytes ensures database block parsing strips trailing bytes
+// from the cached serialization, passes exact serializations through
+// untouched, and still rejects truncated blocks.
+func TestDBBlockFromBytes(t *testing.T) {
+	t.Parallel()
+
+	params := &chaincfg.MainNetParams
+	var serialized bytes.Buffer
+	err := params.GenesisBlock.Serialize(&serialized)
+	if err != nil {
+		t.Fatalf("failed to serialize block: %v", err)
+	}
+	cleanBytes := serialized.Bytes()
+	wantHash := params.GenesisBlock.BlockHash()
+
+	// A block with trailing bytes must parse, and the cached
+	// serialization must exclude the trailing data.
+	block, err := DBBlockFromBytes(
+		append(append([]byte(nil), cleanBytes...), 0x00), wantHash,
+	)
+	if err != nil {
+		t.Fatalf("failed to parse block with trailing bytes: %v", err)
+	}
+	gotBytes, err := block.Bytes()
+	if err != nil {
+		t.Fatalf("failed to serialize parsed block: %v", err)
+	}
+	if !bytes.Equal(gotBytes, cleanBytes) {
+		t.Fatal("cached serialization includes trailing bytes")
+	}
+	if *block.Hash() != wantHash {
+		t.Fatalf("unexpected block hash - got %v, want %v",
+			block.Hash(), wantHash)
+	}
+
+	// An exact serialization must pass through untouched.
+	block, err = DBBlockFromBytes(cleanBytes, wantHash)
+	if err != nil {
+		t.Fatalf("failed to parse exact block: %v", err)
+	}
+	gotBytes, err = block.Bytes()
+	if err != nil {
+		t.Fatalf("failed to serialize parsed block: %v", err)
+	}
+	if !bytes.Equal(gotBytes, cleanBytes) {
+		t.Fatal("exact serialization was not preserved")
+	}
+
+	// A truncated block must still fail to parse.
+	_, err = DBBlockFromBytes(cleanBytes[:len(cleanBytes)-1], wantHash)
+	if err == nil {
+		t.Fatal("expected truncated block to fail to parse")
 	}
 }
 

blockchain/indexers/manager.go

@@ -309,7 +309,9 @@ func (m *Manager) Init(chain *blockchain.BlockChain, interrupt <-chan struct{})
 				if err != nil {
 					return err
 				}
-				block, err = btcutil.NewBlockFromBytes(blockBytes)
+				block, err = blockchain.DBBlockFromBytes(
+					blockBytes, *hash,
+				)
 				if err != nil {
 					return err
 				}

psbt/psbt.go

@@ -185,6 +185,13 @@ func NewFromUnsignedTx(tx *wire.MsgTx) (*Packet, error) {
 // argument b64 is true, the passed byte slice is decoded from base64 encoding
 // before processing.
 //
+// The parsing is strict: base64 input must not contain whitespace or any
+// characters outside the RFC4648 standard alphabet, and any data after the
+// packet results in ErrInvalidPsbtFormat. Trailing data is only detected
+// when the reader can report its remaining length without blocking (such as
+// bytes.Reader, or the base64 path); a plain stream is not probed past the
+// packet, so the reader is left positioned directly after it.
+//
 // 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) {
@@ -324,34 +331,50 @@ func NewFromRawBytes(r io.Reader, b64 bool) (*Packet, error) {
 		return nil, err
 	}
 
-	if err := assertFullyConsumed(r); err != nil {
-		return nil, err
+	// Reject any trailing data after the packet when the reader is able
+	// to report it without an additional read. This covers in-memory
+	// readers as well as the decoded base64 path above. Plain streams
+	// are not probed, as a read for EOF could block forever on an open
+	// connection that has already delivered a complete packet.
+	if lr, ok := r.(interface{ Len() int }); ok && lr.Len() > 0 {
+		return nil, ErrInvalidPsbtFormat
 	}
 
 	return &newPsbt, nil
 }
 
+// maxBase64PsbtSize is the maximum number of base64 characters accepted when
+// decoding a PSBT. It is wire.MaxMessagePayload, the largest payload the
+// wire protocol will carry, expanded by the 4/3 base64 encoding overhead. It
+// bounds the memory allocated for a caller-supplied reader before any
+// validation runs.
+const maxBase64PsbtSize = 4 * ((wire.MaxMessagePayload + 2) / 3)
+
 // 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)
+	// Bound the read so an unbounded stream cannot force an arbitrarily
+	// large allocation before validation.
+	encoded, err := io.ReadAll(io.LimitReader(r, maxBase64PsbtSize+1))
 	if err != nil {
 		return nil, err
 	}
+	if len(encoded) > maxBase64PsbtSize {
+		return nil, ErrInvalidPsbtFormat
+	}
 
 	// 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)
+	decoded, err := base64.StdEncoding.Strict().AppendDecode(nil, encoded)
 	if err != nil {
 		return nil, ErrInvalidPsbtFormat
 	}
 
-	return decoded[:n], nil
+	return decoded, nil
 }
 
 // Serialize creates a binary serialization of the referenced Packet struct

psbt/strict_tx_values_test.go

@@ -3,7 +3,10 @@ package psbt
 import (
 	"bytes"
 	"encoding/base64"
+	"errors"
+	"io"
 	"testing"
+	"testing/iotest"
 
 	"github.com/btcsuite/btcd/wire/v2"
 	"github.com/stretchr/testify/require"
@@ -172,6 +175,53 @@ func TestRejectsTrailingDataAfterPacket(t *testing.T) {
 	require.ErrorIs(t, err, ErrInvalidPsbtFormat)
 }
 
+// TestStreamReaderNotProbedPastPacket verifies that a reader that cannot
+// report its remaining length is not read past the end of the packet: the
+// packet parses successfully and any subsequent data remains unread, so
+// parsing never blocks on an open stream.
+func TestStreamReaderNotProbedPastPacket(t *testing.T) {
+	unsignedTx, prevTx := strictnessTxPair(t)
+	rawPacket := strictnessPSBT(
+		t,
+		serializeTxForStrictness(t, unsignedTx, true),
+		serializeTxForStrictness(t, prevTx, false),
+	)
+
+	// io.MultiReader hides the Len method of the underlying bytes.Reader,
+	// mimicking a plain stream.
+	stream := io.MultiReader(bytes.NewReader(
+		append(append([]byte{}, rawPacket...), 0xde, 0xad),
+	))
+
+	_, err := NewFromRawBytes(stream, false)
+	require.NoError(t, err)
+
+	// The bytes following the packet must still be readable from the
+	// stream.
+	trailing, err := io.ReadAll(stream)
+	require.NoError(t, err)
+	require.Equal(t, []byte{0xde, 0xad}, trailing)
+}
+
+// TestRejectsOversizedBase64Packet verifies that base64 input larger than
+// the maximum accepted size is rejected instead of being fully decoded.
+func TestRejectsOversizedBase64Packet(t *testing.T) {
+	oversized := bytes.Repeat([]byte{'A'}, maxBase64PsbtSize+1)
+
+	// The erroring sentinel after the oversized bytes pins the bound
+	// itself: with the size limit in place the reader is never read past
+	// maxBase64PsbtSize+1 bytes, so the sentinel stays untouched. Without
+	// the limit, the full read would surface the sentinel error instead
+	// of ErrInvalidPsbtFormat.
+	stream := io.MultiReader(
+		bytes.NewReader(oversized),
+		iotest.ErrReader(errors.New("read past size bound")),
+	)
+
+	_, err := NewFromRawBytes(stream, true)
+	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) {

psbt/utils.go

@@ -279,27 +279,12 @@ func getKey(r io.Reader) (int, []byte, error) {
 
 // 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
-	}
-
-	var trailing [1]byte
-	_, err := io.ReadFull(r, trailing[:])
-	switch {
-	case err == nil:
+func assertFullyConsumed(r *bytes.Reader) error {
+	if r.Len() > 0 {
 		return ErrInvalidPsbtFormat
-
-	case errors.Is(err, io.EOF):
-		return nil
-
-	default:
-		return err
 	}
+
+	return nil
 }
 
 // readTxOut parses a transaction output value and requires the full value to
@@ -318,9 +303,10 @@ func readTxOut(txout []byte) (*wire.TxOut, error) {
 	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.
+// 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)