Public commit record
78/100 · Adequate
What the developer wrote
Authored by Olaoluwa Osuntokun
psbt: decode base64 packets incrementally
In this commit, we decode base64 PSBT packets as a stream instead of
reading the full encoded input into memory first.
The previous aggregate bound reused wire.MaxMessagePayload, even though
BIP-174 doesn't bind PSBT packet size to the P2P message limit. This made
the base64 path reject packets that the raw path accepted. We now feed
decoded bytes through the same per-field parser used for raw packets,
while retaining strict base64 and trailing-data checks.
We also add a regression test with nine individually valid 4,000,000-byte
unknown values to pin raw and base64 parsing to the same behavior.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit changes how btcd reads base64-encoded PSBT (Partially Signed Bitcoin Transaction) data. Previously, the code read the entire base64 input into memory before decoding, and it wrongly applied the Bitcoin P2P wire message size limit to PSBT packets. That caused the base64 parser to reject large but otherwise valid PSBTs that the raw binary parser would accept. The new code decodes base64 incrementally as a stream, removes the wire-size bound for base64 PSBTs, and still rejects whitespace, non-canonical characters, and trailing data. A regression test confirms that nine 4 MB unknown fields are accepted via both raw and base64 paths.
Recommended action
Review the streaming decoder's error mapping and EOF handling to ensure malformed/truncated base64 cannot produce partial fields or ambiguous errors. Confirm that assertBase64FullyConsumed cannot block on streaming readers and that the canonicalBase64Reader correctly rejects CR/LF across buffer boundaries.
Security signals we found
01
Removes an incorrect size bound that caused base64 PSBT parsing to reject valid large packets (denial-of-service / interoperability issue)
02
Switches from full in-memory base64 decode to incremental streaming, reducing peak memory for large base64 PSBTs
03
Retains strict base64 alphabet checks and rejects trailing data after the PSBT packet
04
Adds regression test pinning raw and base64 parsing behavior for oversized valid packets
Evidence from the diff
The patch replaces the all-at-once base64 decode/decodeBase64Strict in psbt.NewFromRawBytes with a streaming strictBase64Decoder built on base64.NewDecoder. It removes maxBase64PsbtSize and the wire.MaxMessagePayload-derived bound for base64 input, instead feeding decoded bytes through the same per-field parser used for raw packets. It adds a canonicalBase64Reader to reject CR/LF (which Go’s strict decoder otherwise ignores), maps base64 syntax errors to ErrInvalidPsbtFormat, and uses assertBase64FullyConsumed to detect trailing decoded bytes after the PSBT packet. The regression test verifies that a PSBT larger than wire.MaxMessagePayload parses identically through raw and base64 paths.
Changed components
psbt/psbt.gopsbt/strict_tx_values_test.goInspect captured patch +129 / −60
diff --git a/psbt/psbt.go b/psbt/psbt.go
index 5a3f778..4f61858 100644
--- a/psbt/psbt.go
+++ b/psbt/psbt.go
@@ -187,20 +187,17 @@ func NewFromUnsignedTx(tx *wire.MsgTx) (*Packet, error) {
//
// 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.
+// packet results in ErrInvalidPsbtFormat. For raw input, trailing data is only
+// detected when the reader can report its remaining length without blocking
+// (such as bytes.Reader); a plain raw stream is not probed past the packet, so
+// the reader is left positioned directly after it. Base64 input is decoded
+// incrementally and read through its canonical end.
//
// 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 b64 {
- decoded, err := decodeBase64Strict(r)
- if err != nil {
- return nil, err
- }
- r = bytes.NewReader(decoded)
+ r = newStrictBase64Decoder(r)
}
// The Packet struct does not store the fixed magic bytes, but they
@@ -331,50 +328,91 @@ func NewFromRawBytes(r io.Reader, b64 bool) (*Packet, error) {
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
+ if b64 {
+ if err := assertBase64FullyConsumed(r); err != nil {
+ return nil, err
+ }
+ } else {
+ // Reject any trailing data after the packet when a raw reader can
+ // report it without an additional read. Plain raw 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) {
- // 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
+// canonicalBase64Reader rejects the CR and LF bytes that encoding/base64
+// otherwise ignores while decoding.
+type canonicalBase64Reader struct {
+ io.Reader
+}
+
+// Read returns encoded bytes only when they use the canonical RFC4648
+// alphabet.
+func (r *canonicalBase64Reader) Read(p []byte) (int, error) {
+ n, err := r.Reader.Read(p)
+ if bytes.ContainsAny(p[:n], "\r\n") {
+ return 0, 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
+ return n, err
+}
+
+// strictBase64Decoder maps base64 syntax and truncation errors to
+// ErrInvalidPsbtFormat while preserving other errors returned by the
+// caller-supplied reader.
+type strictBase64Decoder struct {
+ io.Reader
+}
+
+// Read returns incrementally decoded bytes.
+func (d *strictBase64Decoder) Read(p []byte) (int, error) {
+ n, err := d.Reader.Read(p)
+ if err == nil || errors.Is(err, io.EOF) {
+ return n, err
}
- decoded, err := base64.StdEncoding.Strict().AppendDecode(nil, encoded)
- if err != nil {
- return nil, ErrInvalidPsbtFormat
+ var corruptInput base64.CorruptInputError
+ if errors.As(err, &corruptInput) ||
+ errors.Is(err, io.ErrUnexpectedEOF) ||
+ errors.Is(err, ErrInvalidPsbtFormat) {
+
+ return n, ErrInvalidPsbtFormat
}
- return decoded, nil
+ return n, err
+}
+
+// newStrictBase64Decoder returns a streaming RFC4648 base64 decoder that
+// rejects whitespace and only accepts '=' as final padding.
+func newStrictBase64Decoder(r io.Reader) io.Reader {
+ canonicalReader := &canonicalBase64Reader{Reader: r}
+ decoder := base64.NewDecoder(
+ base64.StdEncoding.Strict(), canonicalReader,
+ )
+
+ return &strictBase64Decoder{Reader: decoder}
+}
+
+// assertBase64FullyConsumed verifies the end of the base64 envelope and
+// rejects decoded data after the PSBT packet.
+func assertBase64FullyConsumed(r io.Reader) error {
+ var trailing [1]byte
+ _, err := io.ReadFull(r, trailing[:])
+ switch {
+ case err == nil:
+ return ErrInvalidPsbtFormat
+
+ case errors.Is(err, io.EOF):
+ return nil
+
+ default:
+ return err
+ }
}
// 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 9122aa2..25ac2e4 100644
--- a/psbt/strict_tx_values_test.go
+++ b/psbt/strict_tx_values_test.go
@@ -3,10 +3,8 @@ package psbt
import (
"bytes"
"encoding/base64"
- "errors"
"io"
"testing"
- "testing/iotest"
"github.com/btcsuite/btcd/wire/v2"
"github.com/stretchr/testify/require"
@@ -203,23 +201,56 @@ func TestStreamReaderNotProbedPastPacket(t *testing.T) {
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")),
- )
+// TestAcceptsBase64PacketAboveWirePayloadLimit verifies that base64 parsing
+// accepts valid PSBT packets larger than the peer-to-peer wire message payload
+// limit.
+func TestAcceptsBase64PacketAboveWirePayloadLimit(t *testing.T) {
+ unsignedTx, _ := strictnessTxPair(t)
+ packet, err := NewFromUnsignedTx(unsignedTx)
+ require.NoError(t, err)
- _, err := NewFromRawBytes(stream, true)
- require.ErrorIs(t, err, ErrInvalidPsbtFormat)
+ value := bytes.Repeat([]byte{0x01}, MaxPsbtValueLength)
+ for i := range 9 {
+ packet.Unknowns = append(packet.Unknowns, &Unknown{
+ Key: []byte{0xfc, byte(i)},
+ Value: value,
+ })
+ }
+
+ var rawPacket bytes.Buffer
+ require.NoError(t, packet.Serialize(&rawPacket))
+ require.Greater(t, rawPacket.Len(), wire.MaxMessagePayload)
+
+ rawParsed, err := NewFromRawBytes(
+ bytes.NewReader(rawPacket.Bytes()), false,
+ )
+ require.NoError(t, err)
+ require.Len(t, rawParsed.Unknowns, 9)
+
+ encodedReader, encodedWriter := io.Pipe()
+ encodeDone := make(chan error, 1)
+ go func() {
+ encoder := base64.NewEncoder(
+ base64.StdEncoding, encodedWriter,
+ )
+ _, encodeErr := io.Copy(encoder, bytes.NewReader(
+ rawPacket.Bytes(),
+ ))
+ if closeErr := encoder.Close(); encodeErr == nil {
+ encodeErr = closeErr
+ }
+ _ = encodedWriter.CloseWithError(encodeErr)
+ encodeDone <- encodeErr
+ }()
+
+ parsedPacket, parseErr := NewFromRawBytes(encodedReader, true)
+ _ = encodedReader.Close()
+ encodeErr := <-encodeDone
+ if parseErr == nil {
+ require.NoError(t, encodeErr)
+ }
+ require.NoError(t, parseErr)
+ require.Len(t, parsedPacket.Unknowns, 9)
}
// TestRejectsNonCanonicalBase64Packet verifies that base64 PSBT input rejects
Risk score
Our methodology →Why this scored 41/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.