ssh: fix infinite loop on large channel writes due to integer overflow

The internal 'min' helper function in channel.go incorrectly cast the
input data length (int) to uint32 before comparing it with the
maximum packet size. On 64-bit systems, if the data length is a
multiple of 2^32 (approx. 4GB), this cast results in 0.

Consequently, the function returns 0, causing the WriteExtended loop
to spin indefinitely because it attempts to reserve 0 bytes while
the remaining data length is still positive.

This change renames the helper to 'minPayloadSize' to avoid confusion
with the Go 1.21 built-in 'min' and updates the logic to use int64
for comparisons, preventing truncation and the resulting infinite loop.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport.

Fixes golang/go#79567
Fixes CVE-2026-39834

Change-Id: Id5bf81d9f06c7042452acffe1c76580ff878665e
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781663
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: drakkan

ssh/channel.go

@@ -131,11 +131,17 @@ func (r RejectionReason) String() string {
 	return fmt.Sprintf("unknown reason %d", int(r))
 }
 
-func min(a uint32, b int) uint32 {
-	if a < uint32(b) {
-		return a
+// minPayloadSize returns min(limit, length) clamped to a uint32. It is used
+// to compute the size of the next channel data packet from the remaining
+// payload. The comparison is done in int64 because length is an int — on
+// 64-bit systems len(data) can exceed 2^32, and a direct uint32(length)
+// cast would silently truncate to 0 at every multiple of 2^32, causing
+// WriteExtended's loop to spin without making progress.
+func minPayloadSize(limit uint32, length int) uint32 {
+	if int64(length) > int64(limit) {
+		return limit
 	}
-	return uint32(b)
+	return uint32(length)
 }
 
 type channelDirection uint8
@@ -251,7 +257,7 @@ func (ch *channel) WriteExtended(data []byte, extendedCode uint32) (n int, err e
 	ch.writeMu.Unlock()
 
 	for len(data) > 0 {
-		space := min(ch.maxRemotePayload, len(data))
+		space := minPayloadSize(ch.maxRemotePayload, len(data))
 		if space, err = ch.remoteWin.reserve(space); err != nil {
 			return n, err
 		}

ssh/channel_test.go

@@ -0,0 +1,140 @@
+// Copyright 2025 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 (
+	"math/bits"
+	"testing"
+	"time"
+	"unsafe"
+)
+
+func TestMinPayloadSize(t *testing.T) {
+	// 4 GiB (2^32). Declared as a var (not a const) so that int(bigPayload)
+	// is a runtime conversion: a constant conversion would fail to compile
+	// on 32-bit platforms with "constant 4294967296 overflows int". On
+	// 32-bit the value truncates to 0 at runtime, but the is64Bit cases
+	// that reference it are skipped by the runtime check below.
+	var bigPayload int64 = 1 << 32
+
+	tests := []struct {
+		name       string
+		maxPayload uint32
+		dataLen    int
+		want       uint32
+		is64Bit    bool // Flag to run only on 64-bit architectures
+	}{
+		{
+			name:       "Normal Case - Data fits in payload",
+			maxPayload: 32768,
+			dataLen:    1000,
+			want:       1000,
+		},
+		{
+			name:       "Normal Case - Data larger than payload",
+			maxPayload: 32768,
+			dataLen:    50000,
+			want:       32768,
+		},
+		{
+			name:       "Boundary Case - Data zero",
+			maxPayload: 32768,
+			dataLen:    0,
+			want:       0,
+		},
+		{
+			name:       "Overflow Case - Data is exactly 4GB (1<<32)",
+			maxPayload: 32768,
+			dataLen:    int(bigPayload),
+			want:       32768,
+			is64Bit:    true,
+		},
+		{
+			name:       "Overflow Case - Data is 4GB + small amount",
+			maxPayload: 32768,
+			dataLen:    int(bigPayload + 100),
+			want:       32768,
+			is64Bit:    true,
+		},
+	}
+
+	is64Bit := bits.UintSize == 64
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.is64Bit && !is64Bit {
+				t.Skip("Skipping test requiring 64-bit int")
+			}
+			got := minPayloadSize(tt.maxPayload, tt.dataLen)
+			if got != tt.want {
+				t.Errorf("minPayloadSize(%d, %d) = %d; want %d", tt.maxPayload, tt.dataLen, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestWriteExtendedNoInfiniteLoopOnLargeWrite is an end-to-end regression
+// test for the integer-overflow bug in WriteExtended. Before the fix, a
+// write whose len(data) was a multiple of 2^32 caused minPayloadSize to
+// return 0; WriteExtended then spun forever, reserving 0 bytes per
+// iteration and never advancing the data slice.
+//
+// We exercise the real WriteExtended path with a slice whose declared
+// length is exactly 2^32. Allocating 4 GiB is unnecessary: each iteration
+// only reads up to maxRemotePayload bytes from the head of the slice, and
+// the loop blocks in remoteWin.reserve() once the channel window is
+// exhausted — before the slice base advances past the underlying buffer.
+//
+// With the fix, the loop blocks in reserve(); we detect that via
+// waitWriterBlocked(), then close the window to let WriteExtended return.
+// With the bug, the loop never blocks and the test times out.
+//
+//go:nocheckptr
+func TestWriteExtendedNoInfiniteLoopOnLargeWrite(t *testing.T) {
+	if bits.UintSize < 64 {
+		t.Skip("test requires 64-bit int to construct a slice with len >= 2^32")
+	}
+
+	reader, writer, mux := channelPair(t)
+	defer reader.Close()
+	defer writer.Close()
+	defer mux.Close()
+
+	// Sized to hold the full pre-update remote window so that no iteration
+	// reads past the backing buffer before reserve() blocks.
+	backing := make([]byte, channelWindowSize)
+	var bigLen int64 = 1 << 32
+	bigSlice := unsafe.Slice(&backing[0], int(bigLen))
+
+	done := make(chan int, 1)
+	go func() {
+		n, _ := writer.Write(bigSlice)
+		done <- n
+	}()
+
+	blocked := make(chan struct{})
+	go func() {
+		writer.remoteWin.waitWriterBlocked()
+		close(blocked)
+	}()
+
+	select {
+	case <-blocked:
+		// Good — the loop made progress and is now blocked in reserve().
+		// Close the window to let WriteExtended return.
+		writer.remoteWin.close()
+	case <-time.After(2 * time.Second):
+		t.Fatal("WriteExtended did not block in reserve within 2s — minPayloadSize likely returned 0 (integer overflow regression)")
+	}
+
+	select {
+	case n := <-done:
+		if n == 0 {
+			t.Fatalf("WriteExtended returned n=0; expected progress")
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("WriteExtended did not return after closing the window")
+	}
+}