From 6c195c8a97ae3d91a366ebdd7787d5faa64bf42a Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 1 Mar 2026 11:49:28 +0100 Subject: [PATCH] ssh: prevent memory leak when rejecting channels When a server rejects an incoming channel request via NewChannel.Reject, the channel is left in the multiplexer's channel list. Because the channel is never explicitly removed or closed, its internal buffers and sync primitives remain allocated for the lifetime of the SSH connection. A malicious client could exploit this behavior by repeatedly requesting to open channels that are destined to be rejected, causing unbounded memory growth and potentially leading to a Denial of Service (DoS) via resource exhaustion. This change fixes the leak by calling ch.mux.chanList.remove within the Reject method, removing the channel from the list and allowing the garbage collector to reclaim the associated memory immediately. Fixes golang/go#35127 Fixes CVE-2026-3982 Change-Id: Iaa177f5dfd151812dd404e528a4a1c77527a0e29 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781320 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Roland Shoemaker Reviewed-by: Nicholas Husin Reviewed-by: Nicholas Husin --- ssh/channel.go | 12 +++++++++++- ssh/mux_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/ssh/channel.go b/ssh/channel.go index cc0bb7a..00e4ceb 100644 --- a/ssh/channel.go +++ b/ssh/channel.go @@ -530,7 +530,17 @@ func (ch *channel) Reject(reason RejectionReason, message string) error { Language: "en", } ch.decided = true - return ch.sendMessage(reject) + err := ch.sendMessage(reject) + + // Remove the channel from the mux to prevent memory leaks. + // Do not call ch.close() here: no goroutine holds a reference to a + // rejected channel's internal channels (msg, incomingRequests), so + // removing it from chanList is sufficient for GC. Calling close() + // would race with the mux loop goroutine (handlePacket or dropAll), + // causing a panic from closing an already-closed channel. + ch.mux.chanList.remove(ch.localId) + + return err } func (ch *channel) Read(data []byte) (int, error) { diff --git a/ssh/mux_test.go b/ssh/mux_test.go index 21f0ac3..95874fa 100644 --- a/ssh/mux_test.go +++ b/ssh/mux_test.go @@ -825,6 +825,50 @@ func TestMuxChannelWindowDeferredUpdates(t *testing.T) { } } +func TestMuxChannelRejectRemovesFromMux(t *testing.T) { + serverMux, clientMux := muxPair() + defer serverMux.Close() + defer clientMux.Close() + + var wg sync.WaitGroup + t.Cleanup(wg.Wait) + wg.Add(1) + + go func() { + defer wg.Done() + + // The server waits for the channel creation request + newCh, ok := <-serverMux.incomingChannels + if !ok { + t.Error("failed to accept channel") + return + } + ch := newCh.(*channel) + + if serverMux.chanList.getChan(ch.localId) == nil { + t.Errorf("channel %d is not in the chanList before Reject", ch.localId) + } + + if err := ch.Reject(Prohibited, "rejecting this channel"); err != nil { + t.Errorf("Reject failed: %v", err) + } + + if serverMux.chanList.getChan(ch.localId) != nil { + t.Errorf("channel %d is still in the chanList after Reject", ch.localId) + } + }() + + _, _, err := clientMux.OpenChannel("test_leak", nil) + + if err == nil { + t.Fatal("expected an error (channel rejected), but got nil") + } + + if _, ok := err.(*OpenChannelError); !ok { + t.Errorf("expected *OpenChannelError, got: %T", err) + } +} + // Don't ship code with debug=true. func TestDebug(t *testing.T) { if debugMux {