decoder: reject length prefix that exhausts input instead of dropping the bound

[tamirms]

Summary

mergeInputLenAndMaxSize overloads the value 0 to mean two different things — "zero input bytes remaining" and "no size limit configured" — and the second meaning wins. When a variable-length field's 4-byte length prefix is the final bytes of the input, InputLen() returns 0, the merge returns 0, and DecodeString / DecodeOpaque / decodeArray remap that 0 to maxInt32 via if maxSize == 0 { maxSize = maxInt32 }. Both the schema bound and the input-length bound are dropped, so a declared length up to maxInt32 passes the uint(dataLen) > uint(maxSize) check and reaches make([]byte, dataLen) in DecodeFixedOpaque before any payload is read — an oversized allocation driven entirely by an untrusted length prefix.

This is independent of the MaxInputLen reader-boundary work in #31 and the depth changes in #32; the reader-level guard is intact, but the consumer-level bound check deletes itself for this input shape.

Fix

Normalize the schema bound to maxInt32 inside mergeInputLenAndMaxSize, before merging with the remaining input length. A returned 0 can then only mean "zero bytes remaining", so the three call sites drop their now-redundant if maxSize == 0 { maxSize = maxInt32 } blocks and compare against the merged bound directly.

func (d *Decoder) mergeInputLenAndMaxSize(maxSize int) int {
	if maxSize <= 0 {
		maxSize = maxInt32
	}
	if left, ok := d.InputLen(); ok && left < maxSize {
		return max(0, left)
	}
	return maxSize
}

The returned value is now always in [0, maxInt32], so uint(dataLen) > uint(maxSize) can never test against a wrapped-negative bound.

Defense in depth: max(0, left) also clamps a negative InputLen() (a miscounting lenLeft reader) instead of wrapping to a huge uint. The same clamp is added to decodeMap's separate guard so the two length checks can't drift apart.

Tests

New regressions in decode_limits_test.go, each verified to fail before this change and pass after (before the fix they are rejected only later, in DecodeFixedOpaqueInplace/DecodeUint, i.e. after the allocation/loop is reached):

• TestMaxInputLenPrefixAtEndOfInput — DecodeString, huge and small declared lengths.
• TestMaxInputLenSlicePrefixAtEndOfInput — the non-opaque decodeArray path.
• TestDecodeMapNegativeInputLenClamped — decodeMap with an adversarial negative-Len() reader.

Four existing DecodeString/DecodeOpaque table rows change from ErrIO to ErrOverflow: these are the cases the table already labels "len larger than available bytes", which is now correctly rejected at the bound check rather than surfacing as an I/O error after the allocation.

Verification

• go test -race ./xdr3/ passes; go vet and gofmt clean.
• Built and tested against downstream stellar/go-stellar-sdk (xdr, txnbuild, ingest) via a temporary replace — all pass, no behavioral regressions.

[Copilot]

Pull request overview

Fixes decoder bounds handling when a variable-length prefix exhausts the remaining input.

Changes:

• Preserves zero remaining-input bounds across variable-length decoders.
• Clamps negative remaining lengths.
• Adds regression tests and updates expected overflow errors.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
xdr3/decode.go	Updates length-bound merging and map checks.
xdr3/decode_test.go	Updates expected error classifications.
xdr3/decode_limits_test.go	Adds input-bound regression tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tamirms]

This doesn't apply: bufio.Reader has no Len() int method — its buffered-byte accessor is Buffered() int (see go doc bufio.Reader). The decoder's length source is the lenLeft interface (Len() int), so bufio.Reader does not satisfy it, is never registered as d.l, and InputLen() returns (0, false) for it. The new if left, ok := d.InputLen(); ok && ... branch is skipped entirely, so a chunked bufio.Reader decode is unaffected by this change.

Verified end-to-end: a string whose length prefix and payload arrive in separate Read chunks, wrapped in bufio.NewReader, decodes successfully on both this branch and master — no regression. An instrumented probe confirms bufio.Reader implements lenLeft: NO and InputLen ok=false.

The readers that do satisfy lenLeft — bytes.Reader, bytes.Buffer, strings.Reader — all report total remaining input, which is the documented contract for the MaxInputLen == 0 "infer size from Len()" mode. A reader whose Len() under-reported would already have been mis-handled before this change (it rejects valid data whenever 0 < Len() < dataLen), so this PR introduces no new behavior there.