fix: reject 32-bit length headers that overflow int

.github/workflows/build.yml

@@ -27,3 +27,24 @@ jobs:
         run: make test
         env:
           GOGC: 50
+
+  test-32bit:
+    name: test (linux/386)
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: '1.26.x'
+
+      # ubuntu-latest is amd64, which runs 386 binaries natively -- no
+      # emulation needed. Catches int overflow on 32-bit length headers.
+      - name: Test (32-bit)
+        run: go test ./...
+        env:
+          GOARCH: 386
+          GOGC: 50

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Unreleased
 
+### Fixed
+
+- **decode:** reject 32-bit length headers that overflow `int` on 32-bit platforms — `map32`, `array32`, `str32`, and `bin32` headers declaring a length above `math.MaxInt` were converted with `int(n)`, which on 32-bit builds wraps to a negative value. A length of `0xffffffff` wrapped to `-1` and was silently decoded as a nil map/array/string (wrong result, no error); values in `[0x80000000, 0xfffffffe]` wrapped to other negatives and panicked in `make()` or on a slice expression. All four headers now route through a single checked conversion, and `readN`/`readNGrow` reject negative lengths as defense in depth. 64-bit platforms are unaffected (every `uint32` fits in an `int`). CI now runs the suite under `GOARCH=386`.
+
 ### Performance
 
 - **decode:** reuse caller-supplied destination map for `map[string]interface{}` — `Decode(&m)`/`Unmarshal(data, &m)` with a non-nil `m` now decodes into the existing map (entries merged) instead of replacing it with a fresh allocation, matching the long-standing `map[string]string` behavior. Applies to all decode paths for the type: the `Decode()` fast path, struct fields, and named map types. Callers that `clear(m)` and reuse the destination get zero map allocations per decode ([#61](https://github.com/Basekick-Labs/msgpack/issues/61)) (decoding a 4-key map into a reused destination, v6 vs this change on the same benchmark: **-22.9% ns/op**, **-80.8% B/op**, 12 → 10 allocs/op). Note: this diverges from upstream, which replaces a non-nil `map[string]interface{}` destination; pass a nil map to keep replace semantics.

Makefile

@@ -3,4 +3,10 @@ test:
 	go test ./... -short -race -count=1 -timeout=5m
 	go test ./... -run=NONE -bench=. -benchmem
 	env GOOS=linux GOARCH=386 go vet ./...
+	# 386 binaries run natively on amd64 hosts (CI); skipped elsewhere.
+	@if [ "$$(go env GOHOSTOS)/$$(go env GOHOSTARCH)" = "linux/amd64" ]; then \
+		echo "env GOARCH=386 go test ./..."; env GOARCH=386 go test ./...; \
+	else \
+		echo "skipping 32-bit test run (needs linux/amd64 host)"; \
+	fi
 	go vet

decode.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"reflect"
 	"sync"
 	"time"
@@ -733,9 +734,12 @@ func (d *Decoder) readFull(b []byte) error {
 }
 
 func (d *Decoder) readN(n int) ([]byte, error) {
+	if n < 0 {
+		return nil, fmt.Errorf("msgpack: invalid length %d", n)
+	}
 	// Fast path: byte-slice reader — zero-copy sub-slice of input buffer.
 	if d.bsr.data != nil {
-		if d.bsr.pos+n > len(d.bsr.data) {
+		if n > len(d.bsr.data)-d.bsr.pos {
 			return nil, io.ErrUnexpectedEOF
 		}
 		b := d.bsr.data[d.bsr.pos : d.bsr.pos+n]
@@ -762,6 +766,9 @@ func (d *Decoder) readN(n int) ([]byte, error) {
 }
 
 func readN(r io.Reader, b []byte, n int) ([]byte, error) {
+	if n < 0 {
+		return nil, fmt.Errorf("msgpack: invalid length %d", n)
+	}
 	if b == nil {
 		if n == 0 {
 			return make([]byte, 0), nil
@@ -780,6 +787,9 @@ func readN(r io.Reader, b []byte, n int) ([]byte, error) {
 }
 
 func readNGrow(r io.Reader, b []byte, n int) ([]byte, error) {
+	if n < 0 {
+		return nil, fmt.Errorf("msgpack: invalid length %d", n)
+	}
 	if b == nil {
 		if n == 0 {
 			return make([]byte, 0), nil
@@ -826,3 +836,23 @@ func min(a, b int) int { //nolint:unparam
 	}
 	return b
 }
+
+// uint32Len converts a length decoded from a 32-bit msgpack header to int.
+//
+// On 64-bit platforms every uint32 fits in an int and this is a no-op. On
+// 32-bit platforms a value above math.MaxInt32 would wrap to a negative
+// int, which callers treat as "nil" (-1) or pass to make(), so it is
+// rejected instead. err is threaded through so call sites stay one-liners.
+func uint32Len(n uint32, err error, what string) (int, error) {
+	if err != nil {
+		return 0, err
+	}
+	if uint64(n) > uint64(maxLenForInt) {
+		return 0, fmt.Errorf("msgpack: %s length %d overflows int", what, n)
+	}
+	return int(n), nil
+}
+
+// maxLenForInt is math.MaxInt, in a variable so tests can lower it to
+// math.MaxInt32 and exercise the 32-bit rejection path on a 64-bit host.
+var maxLenForInt uint64 = math.MaxInt

decode_map.go

@@ -108,7 +108,7 @@ func (d *Decoder) mapLen(c byte) (int, error) {
 	}
 	if c == msgpcode.Map32 {
 		size, err := d.uint32()
-		return int(size), err
+		return uint32Len(size, err, "map")
 	}
 	return 0, unexpectedCodeError{code: c, hint: "map length"}
 }

decode_slice.go

@@ -33,7 +33,7 @@ func (d *Decoder) arrayLen(c byte) (int, error) {
 		return int(n), err
 	case msgpcode.Array32:
 		n, err := d.uint32()
-		return int(n), err
+		return uint32Len(n, err, "array")
 	}
 	return 0, fmt.Errorf("msgpack: invalid code=%x decoding array length", c)
 }

decode_string.go

@@ -25,7 +25,7 @@ func (d *Decoder) bytesLen(c byte) (int, error) {
 		return int(n), err
 	case msgpcode.Str32, msgpcode.Bin32:
 		n, err := d.uint32()
-		return int(n), err
+		return uint32Len(n, err, "string/bytes")
 	}
 
 	return 0, fmt.Errorf("msgpack: invalid code=%x decoding string/bytes length", c)

overflow_test.go

@@ -0,0 +1,162 @@
+package msgpack
+
+import (
+	"math"
+	"strings"
+	"testing"
+)
+
+// uint32Len is the single conversion point for 32-bit msgpack length
+// headers. On 64-bit platforms every uint32 fits in an int; on 32-bit
+// platforms values above math.MaxInt32 must be rejected rather than
+// wrapped to a negative int (which callers would read as nil, or pass to
+// make()). The table drives uint32Len directly so both behaviors are
+// covered regardless of the host word size.
+func TestUint32LenOverflow(t *testing.T) {
+	tests := []struct {
+		name    string
+		n       uint32
+		wantErr bool
+	}{
+		{name: "zero", n: 0},
+		{name: "small", n: 42},
+		{name: "maxint32", n: math.MaxInt32},
+		// Wraps to a negative int on 32-bit; fine on 64-bit.
+		{name: "maxint32+1", n: math.MaxInt32 + 1, wantErr: intIs32Bit},
+		// Wraps to exactly -1 on 32-bit -- the "silently decodes as nil" case.
+		{name: "maxuint32", n: math.MaxUint32, wantErr: intIs32Bit},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := uint32Len(tt.n, nil, "test")
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("uint32Len(%d) = %d, want overflow error", tt.n, got)
+				}
+				if !strings.Contains(err.Error(), "overflows int") {
+					t.Fatalf("uint32Len(%d) err = %v, want overflow error", tt.n, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("uint32Len(%d) unexpected error: %v", tt.n, err)
+			}
+			if got < 0 {
+				t.Fatalf("uint32Len(%d) = %d, must never be negative", tt.n, got)
+			}
+			if uint32(got) != tt.n {
+				t.Fatalf("uint32Len(%d) = %d, want %d", tt.n, got, tt.n)
+			}
+		})
+	}
+}
+
+const intIs32Bit = math.MaxInt == math.MaxInt32
+
+// An existing error is passed through untouched.
+func TestUint32LenPropagatesError(t *testing.T) {
+	sentinel := errTestSentinel
+	if _, err := uint32Len(math.MaxUint32, sentinel, "test"); err != sentinel {
+		t.Fatalf("uint32Len err = %v, want the incoming error", err)
+	}
+}
+
+var errTestSentinel = errSentinel{}
+
+type errSentinel struct{}
+
+func (errSentinel) Error() string { return "sentinel" }
+
+// Negative lengths must never reach make() or a slice expression.
+func TestReadNRejectsNegative(t *testing.T) {
+	d := NewDecoder(strings.NewReader("whatever"))
+	if _, err := d.readN(-1); err == nil {
+		t.Fatal("d.readN(-1) = nil error, want invalid length error")
+	}
+	if _, err := readN(strings.NewReader("x"), nil, -1); err == nil {
+		t.Fatal("readN(-1) = nil error, want invalid length error")
+	}
+	if _, err := readNGrow(strings.NewReader("x"), nil, -1); err == nil {
+		t.Fatal("readNGrow(-1) = nil error, want invalid length error")
+	}
+}
+
+// Forces the 32-bit rejection path on any host by lowering the ceiling to
+// math.MaxInt32, so CI proves the guard works without a 32-bit runner.
+func TestUint32LenRejectsOverflowAs32Bit(t *testing.T) {
+	orig := maxLenForInt
+	maxLenForInt = math.MaxInt32
+	defer func() { maxLenForInt = orig }()
+
+	for _, n := range []uint32{math.MaxInt32 + 1, 0x80000000, math.MaxUint32} {
+		got, err := uint32Len(n, nil, "test")
+		if err == nil {
+			t.Fatalf("uint32Len(%#x) = %d, want overflow error when int is 32-bit", n, got)
+		}
+	}
+	// Values that still fit must keep working.
+	if got, err := uint32Len(math.MaxInt32, nil, "test"); err != nil || got != math.MaxInt32 {
+		t.Fatalf("uint32Len(MaxInt32) = %d, %v; want %d, nil", got, err, int64(math.MaxInt32))
+	}
+}
+
+// End-to-end: malicious 32-bit headers decoded through the public API with
+// the ceiling forced to 32-bit. Each must return an error rather than
+// panicking in make() or silently decoding as nil.
+func TestDecodeRejectsOverflowingHeadersAs32Bit(t *testing.T) {
+	orig := maxLenForInt
+	maxLenForInt = math.MaxInt32
+	defer func() { maxLenForInt = orig }()
+
+	tests := []struct {
+		name string
+		data []byte
+		dst  func() interface{}
+	}{
+		{
+			name: "map32 maxuint32",
+			data: []byte{0xdf, 0xff, 0xff, 0xff, 0xff},
+			dst:  func() interface{} { return &map[string]interface{}{} },
+		},
+		{
+			name: "array32 maxuint32",
+			data: []byte{0xdd, 0xff, 0xff, 0xff, 0xff},
+			dst:  func() interface{} { return &[]interface{}{} },
+		},
+		{
+			name: "str32 maxuint32",
+			data: []byte{0xdb, 0xff, 0xff, 0xff, 0xff},
+			dst:  func() interface{} { var s string; return &s },
+		},
+		{
+			name: "bin32 maxuint32",
+			data: []byte{0xc6, 0xff, 0xff, 0xff, 0xff},
+			dst:  func() interface{} { var b []byte; return &b },
+		},
+		{
+			name: "str32 0x80000000",
+			data: []byte{0xdb, 0x80, 0x00, 0x00, 0x00},
+			dst:  func() interface{} { var s string; return &s },
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defer func() {
+				if r := recover(); r != nil {
+					t.Fatalf("panic decoding %s: %v", tt.name, r)
+				}
+			}()
+			err := Unmarshal(tt.data, tt.dst())
+			if err == nil {
+				t.Fatalf("Unmarshal(%s) = nil error, want overflow rejection", tt.name)
+			}
+			// Must be rejected at the length check, not incidentally by a
+			// later EOF -- that distinction is the whole point of the guard.
+			if !strings.Contains(err.Error(), "overflows int") {
+				t.Fatalf("Unmarshal(%s) err = %v, want an \"overflows int\" rejection", tt.name, err)
+			}
+		})
+	}
+}