From a3bed5e308999f28e8c16d4692f488511d04b85d Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 13 Jul 2026 20:54:04 -0700 Subject: [PATCH 1/3] blockchain: tolerate trailing bytes when loading stored blocks In this commit, we relax the strict block deserialization introduced as part of the trailing byte hardening. Databases written by older versions of btcd may have persisted blocks with trailing bytes, so refusing to load them would prevent a node from ever starting (or serving such a block) after an upgrade, with no recovery path short of a full resync. We instead introduce a new dbBlockFromBytes helper, used by both initChainState and dbFetchBlockByNode, that deserializes the block leniently: any trailing bytes are logged, ignored, and excluded from the serialization cached on the returned block, so downstream consumers of the raw bytes never observe them. --- blockchain/chainio.go | 44 +++++++++++++++++-- blockchain/chainio_test.go | 86 ++++++++++++++++++++++++++++++++++---- 2 files changed, 117 insertions(+), 13 deletions(-) diff --git a/blockchain/chainio.go b/blockchain/chainio.go index 9ce58b1232..9d183a7238 100644 --- a/blockchain/chainio.go +++ b/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 } diff --git a/blockchain/chainio_test.go b/blockchain/chainio_test.go index 272bf61d67..7fb961b49b 100644 --- a/blockchain/chainio_test.go +++ b/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") } } From 2ddf73f39e72cfd57df0a5862434782b8a25cc8c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 13 Jul 2026 20:54:15 -0700 Subject: [PATCH 2/3] psbt: avoid blocking reads and unbounded allocations in NewFromRawBytes In this commit, we address two issues with the strict parsing recently added to NewFromRawBytes. First, the trailing data check probed the caller supplied reader with a blocking one byte read. A reader without a Len method (net.Conn, io.Pipe) that stays open after delivering a complete packet would hang the parser forever. We now only enforce the check when the reader can report its remaining length without an additional read, which covers in-memory readers along with the decoded base64 path. Plain streams are left positioned directly after the packet, and the reader contract is now documented on NewFromRawBytes. Second, the base64 path read the entire input into memory before any validation ran, so a very large input could force an arbitrarily large allocation before the first validity check. We now bound the read to wire.MaxMessagePayload expanded by the base64 encoding overhead. Along the way, we simplify assertFullyConsumed down to the bytes.Reader case that all remaining callers use. --- psbt/psbt.go | 35 +++++++++++++++++++----- psbt/strict_tx_values_test.go | 50 +++++++++++++++++++++++++++++++++++ psbt/utils.go | 30 ++++++--------------- 3 files changed, 87 insertions(+), 28 deletions(-) diff --git a/psbt/psbt.go b/psbt/psbt.go index d8e39f8ce5..5a3f77831f 100644 --- a/psbt/psbt.go +++ b/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,20 +331,37 @@ 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. @@ -345,13 +369,12 @@ func decodeBase64Strict(r io.Reader) ([]byte, error) { 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 diff --git a/psbt/strict_tx_values_test.go b/psbt/strict_tx_values_test.go index 83ed6bdd20..9122aa2d90 100644 --- a/psbt/strict_tx_values_test.go +++ b/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) { diff --git a/psbt/utils.go b/psbt/utils.go index baf7558304..289e596c23 100644 --- a/psbt/utils.go +++ b/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) From 97d925ac5f8bbd9465ed26305bb58708c5f3d972 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 13 Jul 2026 21:17:44 -0700 Subject: [PATCH 3/3] multi: parse own-DB blocks leniently in getblock and indexer init In this commit, we extend the lenient database block parsing to the two remaining call sites that re-parse blocks read back from the node's own database with the strict btcutil.NewBlockFromBytes. The getblock RPC fetched the raw block bytes directly, so a legacy block with trailing bytes would fail with an internal error at verbosity >= 1, while verbosity 0 served the dirty bytes (which any hardened consumer now rejects) and reported a size that included them. We now parse via the newly exported blockchain.DBBlockFromBytes and serve the exact block serialization at all verbosity levels. The index manager's init path loads orphaned blocks directly from the database when rolling an index tip back to the main chain, and would fail startup permanently on a legacy block with trailing bytes. It now uses the same lenient parse. --- blockchain/indexers/manager.go | 4 +++- rpcserver.go | 22 +++++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/blockchain/indexers/manager.go b/blockchain/indexers/manager.go index 28f608fbe1..533f007396 100644 --- a/blockchain/indexers/manager.go +++ b/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 } diff --git a/rpcserver.go b/rpcserver.go index fb5f0665e3..4182549d5f 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1092,6 +1092,21 @@ func handleGetBlock(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (i Message: "Block not found", } } + // Deserialize the block. Blocks read back from the node's own + // database are parsed leniently: trailing bytes persisted by older + // btcd versions are stripped rather than treated as an error, so the + // bytes served below are always the exact block serialization. + blk, err := blockchain.DBBlockFromBytes(blkBytes, *hash) + if err != nil { + context := "Failed to deserialize block" + return nil, internalRPCError(err.Error(), context) + } + blkBytes, err = blk.Bytes() + if err != nil { + context := "Failed to serialize block" + return nil, internalRPCError(err.Error(), context) + } + // If verbosity is 0, return the serialized block as a hex encoded string. if c.Verbosity != nil && *c.Verbosity == 0 { return hex.EncodeToString(blkBytes), nil @@ -1099,13 +1114,6 @@ func handleGetBlock(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (i // Otherwise, generate the JSON object and return it. - // Deserialize the block. - blk, err := btcutil.NewBlockFromBytes(blkBytes) - if err != nil { - context := "Failed to deserialize block" - return nil, internalRPCError(err.Error(), context) - } - // Get the block height from chain. blockHeight, err := s.cfg.Chain.BlockHeightByHash(hash) if err != nil {