[crypto] ssh: drop unexpected message types in channel.handlePacket
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded the change for review![]( "Open in Gerrit")
Commit message
ssh: drop unexpected message types in channel.handlePacket
`(*channel).handlePacket` handles the channel response and request messages explicitly and falls back to `ch.msg <- msg` in the `default:` arm for every other decoded type. Nothing ever reads those types from `ch.msg`: it is only consumed by `SendRequest` (`channelRequestSuccess`/`Failure`) and the channel-open wait (`channelOpenConfirm`/`Failure`), all handled in explicit cases above.
A peer can route any `decode()`-able message (for example a service-request or ext-info packet) to an existing channel id; it falls through to the default arm and is queued to the bounded `ch.msg`. On an open, idle channel nothing drains `ch.msg`, so after `chanSize` such messages the blocking send in the default arm stalls the single mux read loop for the whole connection (the `mux`/`readLoop` goroutines leak; closing the connection does not release them). When a `SendRequest` is in flight, the queued messages instead corrupt its response handling.
This is the same primitive that was addressed for the unexpected-response case (the fix for the deadlock on unexpected channel responses), which moved `channelRequestSuccess`/`Failure` out of the default arm into a gated, non-blocking case but left the default arm unchanged. Since no consumer awaits any default-arm type, ignore them instead of queuing to `ch.msg`.
Adds a regression test (`TestChannelUnexpectedDefaultMessagesDiscarded`) that floods a channel with default-arm messages and confirms the channel and mux loop remain usable; it fails without the fix. `go test ./ssh/...` passes.
Change diff
diff --git a/ssh/channel.go b/ssh/channel.go
index afc9aef..e9138d6 100644
--- a/ssh/channel.go
+++ b/ssh/channel.go
@@ -486,7 +486,12 @@
default:
}
default:
- ch.msg <- msg
+ // Ignore message types that are not valid on an established channel.
+ // Only the channel-open responses and the SendRequest replies handled
+ // in the cases above are ever read from ch.msg; delivering any other
+ // type there would let a peer fill ch.msg on an open, idle channel and
+ // permanently stall the single mux read loop — the same primitive as
+ // the unexpected-response case fixed above (CVE-2026-39830).
}
return nil
}
diff --git a/ssh/channel_defaultarm_test.go b/ssh/channel_defaultarm_test.go
new file mode 100644
index 0000000..e020354
--- /dev/null
+++ b/ssh/channel_defaultarm_test.go
@@ -0,0 +1,98 @@
+// Copyright 2026 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ssh
+
+import (
+ "encoding/binary"
+ "errors"
+ "fmt"
+ "io"
+ "testing"
+)
+
+// TestChannelUnexpectedDefaultMessagesDiscarded is the default-arm counterpart
+// of TestChannelUnexpectedResponsesDiscarded. A peer that spams messages which
+// fall through to the default arm of (*channel).handlePacket — e.g. a
+// service-request packet routed to a channel id — must not be able to fill
+// ch.msg on an open, idle channel and stall the single mux read loop.
+func TestChannelUnexpectedDefaultMessagesDiscarded(t *testing.T) {
+ clientMux, serverMux := muxPair()
+ defer serverMux.Close()
+ defer clientMux.Close()
+
+ serverRes := make(chan *channel, 1)
+ go func() {
+ newCh, ok := <-serverMux.incomingChannels
+ if !ok {
+ close(serverRes)
+ return
+ }
+ c, _, err := newCh.Accept()
+ if err != nil {
+ close(serverRes)
+ return
+ }
+ serverRes <- c.(*channel)
+ }()
+
+ clientCh, err := clientMux.openChannel("chan", nil)
+ if err != nil {
+ t.Fatalf("openChannel: %v", err)
+ }
+ serverCh := <-serverRes
+ if serverCh == nil {
+ t.Fatal("server did not accept channel")
+ }
+
+ // Craft a packet that the client mux routes to clientCh (the channel id is
+ // read from packet[1:5]) and that decode() turns into a *serviceRequestMsg,
+ // hitting the default arm of handlePacket. For a serviceRequestMsg the bytes
+ // after the type byte are a length-prefixed string, so packet[1:5] (the
+ // string length) doubles as the routed channel id.
+ targetID := serverCh.remoteId // == clientCh.localId
+ pkt := []byte{msgServiceRequest}
+ pkt = binary.BigEndian.AppendUint32(pkt, targetID)
+ pkt = append(pkt, make([]byte, targetID)...)
+
+ // More than chanSize so ch.msg would overflow without the default-arm drop.
+ const spam = chanSize * 4
+ done := make(chan error, 1)
+ go func() {
+ for i := 0; i < spam; i++ {
+ if err := serverMux.conn.writePacket(pkt); err != nil {
+ done <- fmt.Errorf("writePacket %d: %w", i, err)
+ return
+ }
+ }
+ // Echo any legitimate request back.
+ for req := range serverCh.incomingRequests {
+ if req.WantReply {
+ if err := req.Reply(true, append([]byte("reply:"), req.Payload...)); err != nil {
+ done <- fmt.Errorf("reply: %w", err)
+ return
+ }
+ }
+ }
+ done <- nil
+ }()
+
+ // If the flood had wedged the client mux read loop, this SendRequest would
+ // never receive a reply.
+ ok, err := clientCh.SendRequest("ping", true, []byte("hello"))
+ if err != nil {
+ t.Fatalf("SendRequest: %v", err)
+ }
+ if !ok {
+ t.Fatal("expected success reply")
+ }
+
+ clientCh.Close()
+ serverCh.Close()
+ if err := <-done; err != nil {
+ if !errors.Is(err, io.EOF) {
+ t.Fatal(err)
+ }
+ }
+}
Change information
- M ssh/channel.go
- A ssh/channel_defaultarm_test.go
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Gopher Robot added 1 comment![]( "Open in Gerrit")
I spotted some possible problems with your PR:
1. You have a long 555 character line in the commit message body. Please add line breaks to long lines that should be wrapped. Lines in the commit message body should be wrapped at ~76 characters unless needed for things like URLs or tables. (Note: GitHub might render long lines as soft-wrapped, so double-check in the Gerrit commit message shown above.)
2. You usually need to reference a bug number for all but trivial or cosmetic fixes. For the crypto repo, the format is usually 'Fixes golang/go#12345' or 'Updates golang/go#12345' at the end of the commit message. Should you have a bug reference?
Please address any problems by updating the GitHub PR.
When complete, mark this comment as 'Done' and click the [blue 'Reply' button](https://go.dev/wiki/GerritBot#i-left-a-reply-to-a-comment-in-gerrit-but-no-one-but-me-can-see-it) above. These findings are based on heuristics; if a finding does not apply, briefly reply here saying so.
To update the commit title or commit message body shown here in Gerrit, you must edit the GitHub PR title and PR description (the first comment) in the GitHub web interface using the 'Edit' button or 'Edit' menu entry there. Note: pushing a new commit to the PR will not automatically update the commit message used by Gerrit.
For more details, see:
- [how to update commit messages](https://go.dev/wiki/GerritBot/#how-does-gerritbot-determine-the-final-commit-message) for PRs imported into Gerrit.
- the Go project's [conventions for commit messages](https://go.dev/doc/contribute#commit_messages) that you should follow.
(In general for Gerrit code reviews, the change author is expected to [log in to Gerrit](https://go-review.googlesource.com/login/) with a Gmail or other Google account and then close out each piece of feedback by marking it as 'Done' if implemented as suggested or otherwise reply to each review comment. See the [Review](https://go.dev/doc/contribute#review) section of the Contributing Guide for details.)
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Adil Burak ŞEN (Gerrit)
Adil Burak ŞEN added 1 comment![]( "Open in Gerrit")
Message
Done. Addressed both points in the updated PR: the commit message body is now wrapped at ~76 characters, and added `Updates golang/go#79564` — this change completes the fix for that issue (CVE-2026-39830), which moved channelRequestSuccess/Failure out of the default arm but left the default arm's unconditional send in place. GerritBot should re-import the revised commit message on its next sync.
1 comment
I spotted some possible problems with your PR:
1. You have a long 555 character line in the commit message body. Please add line breaks to long lines that should be wrapped. Lines in the commit message body should be wrapped at ~76 characters unless needed for things like URLs or tables. (Note: GitHub might render long lines as soft-wrapped, so double-check in the Gerrit commit message shown above.)
2. You usually need to reference a bug number for all but trivial or cosmetic fixes. For the crypto repo, the format is usually 'Fixes golang/go#12345' or 'Updates golang/go#12345' at the end of the commit message. Should you have a bug reference?Please address any problems by updating the GitHub PR.
When complete, mark this comment as 'Done' and click the [blue 'Reply' button](https://go.dev/wiki/GerritBot#i-left-a-reply-to-a-comment-in-gerrit-but-no-one-but-me-can-see-it) above. These findings are based on heuristics; if a finding does not apply, briefly reply here saying so.
To update the commit title or commit message body shown here in Gerrit, you must edit the GitHub PR title and PR description (the first comment) in the GitHub web interface using the 'Edit' button or 'Edit' menu entry there. Note: pushing a new commit to the PR will not automatically update the commit message used by Gerrit.
For more details, see:
- [how to update commit messages](https://go.dev/wiki/GerritBot/#how-does-gerritbot-determine-the-final-commit-message) for PRs imported into Gerrit.
- the Go project's [conventions for commit messages](https://go.dev/doc/contribute#commit_messages) that you should follow.
(In general for Gerrit code reviews, the change author is expected to [log in to Gerrit](https://go-review.googlesource.com/login/) with a Gmail or other Google account and then close out each piece of feedback by marking it as 'Done' if implemented as suggested or otherwise reply to each review comment. See the [Review](https://go.dev/doc/contribute#review) section of the Contributing Guide for details.)
Done. Addressed both points in the updated PR: the commit message body is now wrapped at ~76 characters, and added `Updates golang/go#79564` — this change completes the fix for that issue (CVE-2026-39830), which moved channelRequestSuccess/Failure out of the default arm but left the default arm's unconditional send in place. GerritBot should re-import the revised commit message on its next sync.
Related details
- Nicola Murino
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
Related details
- Nicola Murino
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicola Murino (Gerrit)
Nicola Murino added 1 comment![]( "Open in Gerrit")
Thanks for the CL. We are already working on this exact issue
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Adil Burak ŞEN (Gerrit)
Message from Adil Burak ŞEN![]( "Open in Gerrit")
Thanks for confirming. Happy to leave the regression test (TestChannelUnexpectedDefaultMessagesDiscarded) here if it's useful for your fix; otherwise feel free to close this CL in favour of yours. Glad the default-arm sibling is being addressed.
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |