From ffadf78b2ee9710cffa29ac0e44c026544649a1b Mon Sep 17 00:00:00 2001
From: Arpit Jain <arpitjain099@gmail.com>
Date: Mon, 20 Jul 2026 02:33:04 +0900
Subject: [PATCH] Fix integer overflow in parseString bounds check

parseString read a uint32 length prefix and then checked len(in) against
4+length. That addition is done in uint32, so a length near the maximum
wraps: with length 0xFFFFFFFF, 4+length becomes 3, the guard passes for
any buffer of 4+ bytes, and the code slices in[4:4+length] = in[4:3],
which panics with a slice-bounds error.

This is reachable through a malformed pty-req (parsePtyRequest calls
parseString on the term field). There is no recover() in the package and
the session handler runs in its own goroutine, so the panic takes down
the whole process. On servers with no PublicKeyHandler or PasswordHandler
set, gliderlabs enables NoClientAuth, so this is unauthenticated there.

Advance past the length prefix before the bounds check and compare
len(in) against length directly, matching the shape used in
golang.org/x/crypto/ssh. Valid parsing is unchanged.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
---
 util.go      |  7 ++++---
 util_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)
 create mode 100644 util_test.go

diff --git a/util.go b/util.go
index 015a44ec..4bc1ad8b 100644
--- a/util.go
+++ b/util.go
@@ -66,11 +66,12 @@ func parseString(in []byte) (out string, rest []byte, ok bool) {
 		return
 	}
 	length := binary.BigEndian.Uint32(in)
-	if uint32(len(in)) < 4+length {
+	in = in[4:]
+	if uint32(len(in)) < length {
 		return
 	}
-	out = string(in[4 : 4+length])
-	rest = in[4+length:]
+	out = string(in[:length])
+	rest = in[length:]
 	ok = true
 	return
 }
diff --git a/util_test.go b/util_test.go
new file mode 100644
index 00000000..3c43b293
--- /dev/null
+++ b/util_test.go
@@ -0,0 +1,44 @@
+package ssh
+
+import (
+	"encoding/binary"
+	"testing"
+)
+
+func TestParseStringValid(t *testing.T) {
+	in := make([]byte, 4+5)
+	binary.BigEndian.PutUint32(in, 5)
+	copy(in[4:], []byte("hello"))
+	out, rest, ok := parseString(in)
+	if !ok {
+		t.Fatal("expected ok for a well-formed string")
+	}
+	if out != "hello" {
+		t.Fatalf("got %q, want %q", out, "hello")
+	}
+	if len(rest) != 0 {
+		t.Fatalf("got %d bytes of rest, want 0", len(rest))
+	}
+}
+
+func TestParseStringLengthTooLarge(t *testing.T) {
+	// length prefix of 0xFFFFFFFF. In the old code 4+length overflowed the
+	// uint32 to 3, defeating the bounds check and slicing in[4:3].
+	in := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00}
+	out, rest, ok := parseString(in)
+	if ok {
+		t.Fatalf("expected ok=false for oversized length, got out=%q", out)
+	}
+	if rest != nil {
+		t.Fatalf("expected nil rest, got %v", rest)
+	}
+}
+
+func TestParsePtyRequestOversizedTerm(t *testing.T) {
+	// A pty-req whose term-string length is 0xFFFFFFFF must be rejected, not panic.
+	payload := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00}
+	_, ok := parsePtyRequest(payload)
+	if ok {
+		t.Fatal("expected ok=false for malformed pty-req")
+	}
+}