From 6435c37ab681759aff37ba751d0f2238b3043767 Mon Sep 17 00:00:00 2001 From: Neal Patel Date: Tue, 19 May 2026 20:30:03 -0400 Subject: [PATCH] ssh: sanitize client disconnect messages A malicious peer can send control characters and escape sequences in SSH_MSG_DISCONNECT; strip these characters and %q escape it. Change-Id: I266f78630a7a816a79ce4dda568ce57d7c3f6fb0 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/780180 Commit-Queue: Nicola Murino LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Neal Patel Reviewed-by: Nicola Murino Reviewed-by: Dmitri Shuralyov Auto-Submit: Nicola Murino --- ssh/messages.go | 11 ++++++++++- ssh/messages_test.go | 30 ++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/ssh/messages.go b/ssh/messages.go index ab22c3d..de86f71 100644 --- a/ssh/messages.go +++ b/ssh/messages.go @@ -44,7 +44,16 @@ type disconnectMsg struct { } func (d *disconnectMsg) Error() string { - return fmt.Sprintf("ssh: disconnect, reason %d: %s", d.Reason, d.Message) + return fmt.Sprintf("ssh: disconnect, reason %d: %q", d.Reason, sanitizeString(d.Message)) +} + +func sanitizeString(s string) string { + return strings.Map(func(r rune) rune { + if r == '\t' || (r >= ' ' && r < 0x7f) { + return r + } + return -1 + }, s) } // See RFC 4253, section 7.1. diff --git a/ssh/messages_test.go b/ssh/messages_test.go index ed34628..90f2f4a 100644 --- a/ssh/messages_test.go +++ b/ssh/messages_test.go @@ -9,6 +9,7 @@ import ( "math/big" "math/rand" "reflect" + "strings" "testing" "testing/quick" ) @@ -262,6 +263,31 @@ func TestDecode(t *testing.T) { } } +func TestDisconnectMsgSanitizesMessage(t *testing.T) { + for _, tc := range []struct { + in string + want string + }{ + {"clean message", `"clean message"`}, + {"line1\nline2\nline3", `"line1line2line3"`}, + {"has\x00null\x00bytes", `"hasnullbytes"`}, + {"\x1b[31mred\x1b[0m", `"[31mred[0m"`}, + {"newline\r\nCRLF", `"newlineCRLF"`}, + {"tab\there", `"tab\there"`}, + } { + t.Run(tc.in, func(t *testing.T) { + d := &disconnectMsg{Reason: 11, Message: tc.in} + got := d.Error() + if !strings.Contains(got, tc.want) { + t.Errorf("got %q, want %q", got, tc.want) + } + if bad := "\n\r\x00\x1b"; strings.ContainsAny(got, bad) { + t.Errorf("got %q, want none of %q", got, bad) + } + }) + } +} + func randomBytes(out []byte, rand *rand.Rand) { for i := 0; i < len(out); i++ { out[i] = byte(rand.Int31()) @@ -318,7 +344,7 @@ var ( ) func BenchmarkMarshalKexInitMsg(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Marshal(_kexInitMsg) } } @@ -331,7 +357,7 @@ func BenchmarkUnmarshalKexInitMsg(b *testing.B) { } func BenchmarkMarshalKexDHInitMsg(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Marshal(_kexDHInitMsg) } }