From 4aa4fd834f3df68224b9acaaa7fc47bd1dd48f22 Mon Sep 17 00:00:00 2001 From: "Brandon McGinty (chatgpt)" Date: Sun, 9 Aug 2026 14:21:52 -0400 Subject: [PATCH] Handle minimum signed varint safely --- fix.txt | 4 +-- gumble/gumble/varint/varint_test.go | 22 +++++++++++- gumble/gumble/varint/write.go | 55 +++++++++++++++-------------- 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/fix.txt b/fix.txt index d24395f..018675a 100644 --- a/fix.txt +++ b/fix.txt @@ -211,14 +211,14 @@ Priority 3: configuration, UI, and binding hardening atomic value to 1.0 in New, apply configured volume during connect, allow zero, and save volume changes. -27. Config address and file error handling can panic +[x] 27. Config address and file error handling can panic File: config/user_config.go makeHostPort splits on ':' and panics for malformed addresses or IPv6. fileExists dereferences info after non-ENOENT Stat failures. Replace with net.SplitHostPort (with explicit default-port policy) and return/report errors from Stat rather than dereferencing nil. -28. Config SaveConfig panics and is not robust +[x] 28. Config SaveConfig panics and is not robust File: config/user_config.go Configuration write/rename errors panic the client. Return errors to the caller, preserve the prior config on failure, and consider fsyncing the diff --git a/gumble/gumble/varint/varint_test.go b/gumble/gumble/varint/varint_test.go index 29942ea..ba4bf19 100644 --- a/gumble/gumble/varint/varint_test.go +++ b/gumble/gumble/varint/varint_test.go @@ -1,6 +1,26 @@ package varint // import "git.stormux.org/storm/barnard/gumble/gumble/varint" -import "testing" +import ( + "math" + "testing" +) + +// Regression: MinInt64 formerly caused unbounded recursive encoding, and a +// caller-provided short buffer caused an index panic. +func TestEncodeMinInt64AndShortBuffer(t *testing.T) { + buf := make([]byte, MaxVarintLen) + n := Encode(buf, math.MinInt64) + if n != MaxVarintLen { + t.Fatalf("length = %d, want %d", n, MaxVarintLen) + } + got, consumed := Decode(buf[:n]) + if consumed != n || got != math.MinInt64 { + t.Fatalf("decoded (%d, %d)", got, consumed) + } + if n := Encode(make([]byte, 1), 128); n != 0 { + t.Fatalf("short buffer returned %d", n) + } +} func TestRange(t *testing.T) { diff --git a/gumble/gumble/varint/write.go b/gumble/gumble/varint/write.go index 29d931b..e5a6908 100644 --- a/gumble/gumble/varint/write.go +++ b/gumble/gumble/varint/write.go @@ -9,56 +9,59 @@ import ( // number. const MaxVarintLen = 10 -// Encode encodes the given value to varint format. +// Encode encodes value in the Mumble varint format. It returns zero when b is +// too small, rather than panicking on a caller-provided short buffer. func Encode(b []byte, value int64) int { - // 111111xx Byte-inverted negative two bit number (~xx) + var encoded [MaxVarintLen]byte + n := encode(encoded[:], value) + if n == 0 || len(b) < n { + return 0 + } + copy(b, encoded[:n]) + return n +} + +func encode(b []byte, value int64) int { if value <= -1 && value >= -4 { b[0] = 0xFC | byte(^value&0xFF) return 1 } - // 111110__ + varint Negative recursive varint if value < 0 { b[0] = 0xF8 - return 1 + Encode(b[1:], -value) + // -math.MinInt64 overflows. The decoder intentionally interprets the + // following signed 64-bit payload as MinInt64 and negates it modulo 2^64. + if value == math.MinInt64 { + b[1] = 0xF4 + binary.BigEndian.PutUint64(b[2:], uint64(value)) + return 10 + } + return 1 + encode(b[1:], -value) } - // 0xxxxxxx 7-bit positive number if value <= 0x7F { b[0] = byte(value) return 1 } - // 10xxxxxx + 1 byte 14-bit positive number if value <= 0x3FFF { - b[0] = byte(((value >> 8) & 0x3F) | 0x80) - b[1] = byte(value & 0xFF) + b[0] = byte(value>>8)&0x3F | 0x80 + b[1] = byte(value) return 2 } - // 110xxxxx + 2 bytes 21-bit positive number if value <= 0x1FFFFF { - b[0] = byte((value>>16)&0x1F | 0xC0) - b[1] = byte((value >> 8) & 0xFF) - b[2] = byte(value & 0xFF) + b[0] = byte(value>>16)&0x1F | 0xC0 + b[1], b[2] = byte(value>>8), byte(value) return 3 } - // 1110xxxx + 3 bytes 28-bit positive number if value <= 0xFFFFFFF { - b[0] = byte((value>>24)&0xF | 0xE0) - b[1] = byte((value >> 16) & 0xFF) - b[2] = byte((value >> 8) & 0xFF) - b[3] = byte(value & 0xFF) + b[0] = byte(value>>24)&0x0F | 0xE0 + b[1], b[2], b[3] = byte(value>>16), byte(value>>8), byte(value) return 4 } - // 111100__ + int (32-bit) 32-bit positive number if value <= math.MaxInt32 { b[0] = 0xF0 binary.BigEndian.PutUint32(b[1:], uint32(value)) return 5 } - // 111101__ + long (64-bit) 64-bit number - if value <= math.MaxInt64 { - b[0] = 0xF4 - binary.BigEndian.PutUint64(b[1:], uint64(value)) - return 9 - } - - return 0 + b[0] = 0xF4 + binary.BigEndian.PutUint64(b[1:], uint64(value)) + return 9 }