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

@@ -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

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)