What changed, and why it matters
This commit changes how btcd reads message payloads from the Bitcoin peer-to-peer network. Previously, the code used a standard Go bytes.Buffer, which could reserve more memory than the actual message size after a complete read. The new code reads in bounded, geometrically growing chunks and returns an exactly sized byte slice. The commit message and tests frame this as a memory-budgeting improvement: preventing a complete, well-formed message from retaining hidden extra capacity. It is not a fix for a remote crash or code-execution vulnerability, but it removes a way that a peer could cause a node to hold more memory than the protocol advertises.
Treat as a hardening/memory-budget fix rather than an urgent security patch. Review whether any other buffer growth paths in the wire package can over-allocate relative to advertised lengths, and consider backporting to stable branches if memory DoS is a concern for deployed nodes.
Security signals we found
Memory over-allocation in bytes.Buffer after complete read
Bounded chunk growth capped by remaining payload
New regression test asserts cap(result) == len(result) for complete reads
New regression test asserts boundary errors are preserved for incomplete reads
Benchmarks added for complete framed-message streaming (ping and block)
Evidence from the diff
The readBytes helper in wire/common.go is the central path for reading length-prefixed payloads when the underlying io.Reader does not expose a known remaining size. The old implementation created a bytes.Buffer, grew it to min(count, defaultReadBufferSize), and copied count bytes with io.CopyN. bytes.Buffer.ReadFrom can over-allocate before its final EOF check, so after a complete read the returned []byte could have cap(result) > len(result). The replacement reads bounded chunks starting at defaultReadBufferSize and doubling up to the remaining payload, appending each chunk only after bytes actually arrive. For small messages it returns the single chunk directly; for larger ones it joins chunks with bytes.Join, producing a slice whose capacity equals its length. Tests verify exact capacity after a complete read and correct preservation of a boundary error when the payload is incomplete.
Changed components
wire/common.go readByteswire message decoding path for readers without known remaining lengthBitcoin P2P message deserializationInspect captured patch +152 / −8
### wire/common.go
@@ -43,21 +43,58 @@ func readBytes(r io.Reader, count uint64) ([]byte, error) {
return []byte{}, nil
}
- remaining, known := readerRemaining(r)
- if known && count <= remaining {
+ available, known := readerRemaining(r)
+ if known && count <= available {
result := make([]byte, count)
n, err := io.ReadFull(r, result)
return result[:n], err
}
- var result bytes.Buffer
- result.Grow(int(min(count, defaultReadBufferSize)))
- n, err := io.CopyN(&result, r, int64(count))
- if err == io.EOF && n > 0 {
- err = io.ErrUnexpectedEOF
+ // Read into bounded chunks rather than using io.CopyN with a
+ // bytes.Buffer. Buffer.ReadFrom reserves more space before its final EOF
+ // check, which can grow the buffer beyond count after a complete read.
+ // Each chunk below is capped by the remaining payload and is allocated
+ // only after the preceding bytes arrive.
+ remaining := count
+ chunkSize := uint64(defaultReadBufferSize)
+
+ // The current payload maximum needs fewer than 16 geometrically sized
+ // chunks, so the chunk metadata does not need to grow while reading.
+ chunks := make([][]byte, 0, 16)
+ totalRead := 0
+ for remaining > 0 {
+ chunkSize = min(chunkSize, remaining)
+ chunk := make([]byte, int(chunkSize))
+ n := 0
+ var err error
+ for n < len(chunk) && err == nil {
+ var bytesRead int
+ bytesRead, err = r.Read(chunk[n:])
+ n += bytesRead
+ }
+
+ chunks = append(chunks, chunk[:n])
+ totalRead += n
+ remaining -= uint64(n)
+ if err != nil && remaining > 0 {
+ if err == io.EOF && totalRead > 0 {
+ err = io.ErrUnexpectedEOF
+ }
+ if len(chunks) == 1 {
+ return chunks[0], err
+ }
+
+ return bytes.Join(chunks, nil), err
+ }
+
+ chunkSize *= 2
+ }
+
+ if len(chunks) == 1 {
+ return chunks[0], nil
}
- return result.Bytes(), err
+ return bytes.Join(chunks, nil), nil
}
// readerRemaining returns the number of buffered bytes exposed by the concrete
### wire/decode_budget_bench_test.go
@@ -6,6 +6,7 @@ package wire
import (
"bytes"
+ "os"
"testing"
)
@@ -23,6 +24,56 @@ func benchmarkCountPayload(b *testing.B, prefix []byte, count uint64) []byte {
return payload.Bytes()
}
+func benchmarkCompleteStreamingMessage(b *testing.B, msg Message) {
+ b.Helper()
+
+ var encoded bytes.Buffer
+ _, err := WriteMessageWithEncodingN(
+ &encoded, msg, ProtocolVersion, MainNet, WitnessEncoding,
+ )
+ if err != nil {
+ b.Fatal(err)
+ }
+ payload := encoded.Bytes()
+
+ b.SetBytes(int64(len(payload)))
+ b.ReportAllocs()
+ b.ResetTimer()
+ for i := 0; i < b.N; i++ {
+ reader := &readSizeRecorder{
+ reader: bytes.NewReader(payload),
+ }
+ _, _, _, err := ReadMessageWithEncodingN(
+ reader, ProtocolVersion, MainNet, WitnessEncoding,
+ )
+ if err != nil {
+ b.Fatal(err)
+ }
+ }
+}
+
+// BenchmarkCompleteStreamingMessage measures complete framed messages read
+// from a reader that does not expose its remaining length.
+func BenchmarkCompleteStreamingMessage(b *testing.B) {
+ b.Run("ping", func(b *testing.B) {
+ benchmarkCompleteStreamingMessage(b, NewMsgPing(1))
+ })
+
+ blockBytes, err := os.ReadFile(
+ "testdata/block-00000000000000000021868c2cefc52a480d173c849412fe81c4e5ab806f94ab.blk",
+ )
+ if err != nil {
+ b.Fatal(err)
+ }
+ var block MsgBlock
+ if err := block.Deserialize(bytes.NewReader(blockBytes)); err != nil {
+ b.Fatal(err)
+ }
+ b.Run("block", func(b *testing.B) {
+ benchmarkCompleteStreamingMessage(b, &block)
+ })
+}
+
// BenchmarkTruncatedDecodeBudget measures allocation when a length or element
// count is present without the bytes needed to satisfy it.
func BenchmarkTruncatedDecodeBudget(b *testing.B) {
### wire/decode_budget_test.go
@@ -6,6 +6,7 @@ package wire
import (
"bytes"
+ "errors"
"io"
"testing"
@@ -304,11 +305,66 @@ func TestTruncatedVariableBytesBudget(t *testing.T) {
}
}
+// TestCompleteStreamingBytesCapacity ensures a complete opaque read returns an
+// exactly sized payload instead of retaining capacity beyond its length.
+func TestCompleteStreamingBytesCapacity(t *testing.T) {
+ payload := bytes.Repeat(
+ []byte{0x01}, defaultReadBufferSize+1,
+ )
+ reader := &readSizeRecorder{reader: bytes.NewReader(payload)}
+ result, err := readBytes(reader, uint64(len(payload)))
+ if err != nil {
+ t.Fatalf("unable to read complete payload: %v", err)
+ }
+ if !bytes.Equal(result, payload) {
+ t.Fatal("decoded payload differs from input")
+ }
+ if cap(result) != len(result) {
+ t.Fatalf("payload capacity is %d, want %d", cap(result), len(result))
+ }
+}
+
+// TestStreamingBytesPreservesBoundaryError ensures an error returned with a
+// full internal chunk is not discarded while the payload remains incomplete.
+func TestStreamingBytesPreservesBoundaryError(t *testing.T) {
+ errBoundary := errors.New("chunk boundary")
+ reader := &boundaryErrorReader{
+ payload: bytes.Repeat([]byte{0x01}, defaultReadBufferSize),
+ err: errBoundary,
+ }
+ result, err := readBytes(reader, defaultReadBufferSize+1)
+ if !errors.Is(err, errBoundary) {
+ t.Fatalf("unexpected read error: got %v, want %v", err, errBoundary)
+ }
+ if len(result) != defaultReadBufferSize {
+ t.Fatalf("payload length is %d, want %d", len(result),
+ defaultReadBufferSize)
+ }
+ if reader.reads != 1 {
+ t.Fatalf("reader called %d times, want 1", reader.reads)
+ }
+}
+
type readSizeRecorder struct {
reader io.Reader
max int
}
+type boundaryErrorReader struct {
+ payload []byte
+ err error
+ reads int
+}
+
+func (r *boundaryErrorReader) Read(p []byte) (int, error) {
+ r.reads++
+ if r.reads > 1 {
+ return 0, io.EOF
+ }
+
+ return copy(p, r.payload), r.err
+}
+
type inflatedLenReader struct {
reader io.Reader
length intWhy this scored 49/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.