decoder: enforce MaxInputLen at the io.Reader boundary

xdr3/decode.go

@@ -142,33 +142,28 @@ type lenLeft interface {
 // necessary in complex scenarios where automatic reflection-based decoding
 // won't work.
 type Decoder struct {
-	// used to minimize heap allocations during decoding
+	// scratchBuf minimizes heap allocations during primitive decodes.
+	// rlw is inline so that a decoder with MaxInputLen > 0 pays a single
+	// heap allocation (the Decoder itself) instead of two.
 	scratchBuf     [8]byte
+	rlw            readerLenWrapper
 	r              io.Reader
 	l              lenLeft
 	maxDepth       uint
 	maxMemoryBytes int64
 	memoryBytes    int64
 }
 
-// readerLenWrapper wraps a reader an initial length and provides a Len() method indicating
-// how much input is left
+// readerLenWrapper adapts an io.LimitedReader to the lenLeft interface.
+// io.LimitedReader's Read clamps p and returns io.EOF once N reaches zero,
+// so enforcement happens at the io.Reader boundary regardless of how the
+// underlying reader behaves.
 type readerLenWrapper struct {
-	inner      io.Reader
-	readCount  int
-	initialLen int
+	io.LimitedReader
 }
 
 func (l *readerLenWrapper) Len() int {
-	return l.initialLen - l.readCount
-}
-
-func (l *readerLenWrapper) Read(p []byte) (int, error) {
-	n, err := l.inner.Read(p)
-	if n > 0 {
-		l.readCount += n
-	}
-	return n, err
+	return int(l.N)
 }
 
 // NewDecoder returns a Decoder that can be used to manually decode XDR data
@@ -184,18 +179,28 @@ func NewDecoderWithOptions(r io.Reader, options DecodeOptions) *Decoder {
 	if maxDepth < 1 {
 		maxDepth = DecodeDefaultMaxDepth
 	}
-	mob := options.MaxMemoryBytes
-	if l, ok := r.(lenLeft); ok {
-		return &Decoder{r: r, l: l, maxDepth: maxDepth, maxMemoryBytes: mob}
-	}
+	d := &Decoder{maxDepth: maxDepth, maxMemoryBytes: options.MaxMemoryBytes}
 	if options.MaxInputLen > 0 {
-		rlw := &readerLenWrapper{
-			inner:      r,
-			initialLen: options.MaxInputLen,
+		// Always wrap: enforcement happens at the io.Reader boundary, not
+		// via trusting the caller's lenLeft implementation. Tighten the
+		// budget to the inner reader's reported remaining length when
+		// smaller, so InputLen() reflects actually-readable bytes.
+		budget := int64(options.MaxInputLen)
+		if l, ok := r.(lenLeft); ok {
+			if left := int64(l.Len()); left < budget {
+				budget = left
+			}
 		}
-		return &Decoder{r: rlw, l: rlw, maxDepth: maxDepth, maxMemoryBytes: mob}
+		d.rlw.LimitedReader = io.LimitedReader{R: r, N: budget}
+		d.r = &d.rlw
+		d.l = &d.rlw
+		return d
+	}
+	d.r = r
+	if l, ok := r.(lenLeft); ok {
+		d.l = l
 	}
-	return &Decoder{r: r, l: nil, maxDepth: maxDepth, maxMemoryBytes: mob}
+	return d
 }
 
 // DecodeInt treats the next 4 bytes as an XDR encoded integer and returns the

xdr3/decode_limits_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"io"
 	"testing"
 	"unsafe"
 )
@@ -73,3 +74,93 @@ func TestMaxMemoryBytes(t *testing.T) {
 		}
 	})
 }
+
+// passthroughReader is an io.Reader that does not implement lenLeft, so
+// NewDecoderWithOptions cannot tighten the MaxInputLen budget from the inner
+// reader's remaining length.
+type passthroughReader struct {
+	r *bytes.Reader
+}
+
+func (p *passthroughReader) Read(b []byte) (int, error) { return p.r.Read(b) }
+
+// TestMaxInputLenEnforced verifies that readerLenWrapper refuses reads past
+// the configured MaxInputLen budget even when the underlying reader still has
+// bytes available.
+func TestMaxInputLenEnforced(t *testing.T) {
+	// 4 bytes to fill the budget plus 4 extras the wrapper must refuse.
+	payload := []byte{
+		0x00, 0x00, 0x00, 0x42,
+		0xff, 0xff, 0xff, 0xff,
+	}
+	reader := &passthroughReader{r: bytes.NewReader(payload)}
+
+	dec := NewDecoderWithOptions(reader, DecodeOptions{MaxInputLen: 4})
+
+	if _, _, err := dec.DecodeInt(); err != nil {
+		t.Fatalf("DecodeInt: %v", err)
+	}
+
+	// Inner reader still holds 4 bytes; the wrapper must refuse them.
+	buf := make([]byte, 4)
+	n, err := dec.r.Read(buf)
+	if n != 0 || err != io.EOF {
+		t.Errorf("wrapper Read past budget: got (%d, %v), want (0, io.EOF)", n, err)
+	}
+
+	if left, ok := dec.InputLen(); !ok || left != 0 {
+		t.Errorf("after budget exhaustion: InputLen=(%d,%v), want (0,true)", left, ok)
+	}
+}
+
+// TestMaxInputLenDecodeStringBypass reproduces the reporter PoC end-to-end:
+// a length prefix decoded past the MaxInputLen boundary must not reach the
+// DecodeFixedOpaque allocation phase.
+func TestMaxInputLenDecodeStringBypass(t *testing.T) {
+	payload := []byte{
+		0x00, 0x00, 0x00, 0x42,
+		0x7f, 0xff, 0xff, 0xfb,
+	}
+	reader := &passthroughReader{r: bytes.NewReader(payload)}
+	dec := NewDecoderWithOptions(reader, DecodeOptions{MaxInputLen: 4})
+
+	if _, _, err := dec.DecodeInt(); err != nil {
+		t.Fatalf("DecodeInt: %v", err)
+	}
+
+	if _, _, err := dec.DecodeString(0); err == nil {
+		t.Fatal("DecodeString: expected error, got nil")
+	}
+
+	// Wrapper must refuse the length-prefix read, so the 4 bytes are still
+	// in the inner reader. If the wrapper leaked past budget, the decoder
+	// would have consumed them and proceeded to the allocation phase.
+	if got := reader.r.Len(); got != 4 {
+		t.Errorf("inner reader: got %d bytes remaining, want 4", got)
+	}
+}
+
+// TestMaxInputLenBudget verifies that the wrapper's budget is the minimum of
+// MaxInputLen and the inner reader's reported remaining length, regardless of
+// which is tighter. InputLen() reports the actual initial budget.
+func TestMaxInputLenBudget(t *testing.T) {
+	cases := []struct {
+		name        string
+		payloadLen  int
+		maxInputLen int
+		wantBudget  int
+	}{
+		{"MaxInputLen tighter than inner", 16, 4, 4},
+		{"inner tighter than MaxInputLen", 4, 1 << 20, 4},
+		{"equal", 8, 8, 8},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			reader := bytes.NewReader(make([]byte, tc.payloadLen))
+			dec := NewDecoderWithOptions(reader, DecodeOptions{MaxInputLen: tc.maxInputLen})
+			if left, ok := dec.InputLen(); !ok || left != tc.wantBudget {
+				t.Errorf("InputLen=(%d,%v), want (%d,true)", left, ok, tc.wantBudget)
+			}
+		})
+	}
+}