Skip to content

Commit a162807

Browse files
krutonclaude
andcommitted
fix(security): validate SSH_MSG_CHANNEL_WINDOW_ADJUST to prevent uint32 overflow
A malicious server could send a window adjust that overflows the uint32 maximum (0xFFFFFFFF per RFC 4254 §5.2), causing remoteWindowSize to wrap negative and deadlock the send loop, or allowing unbounded window growth that breaks flow control. Reject adjustments ≤ 0 or that would push the window past the protocol maximum. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b3d0c13 commit a162807

4 files changed

Lines changed: 71 additions & 4 deletions

File tree

sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentChannel.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* ConnectBot SSH Library
3-
* Copyright 2025 Kenny Root
3+
* Copyright 2025-2026 Kenny Root
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818
package org.connectbot.sshlib.client
1919

2020
import kotlinx.coroutines.channels.Channel
21+
import org.connectbot.sshlib.SshException
2122
import org.slf4j.LoggerFactory
2223

2324
internal class AgentChannel(
@@ -30,6 +31,7 @@ internal class AgentChannel(
3031
) {
3132
companion object {
3233
private val logger = LoggerFactory.getLogger(AgentChannel::class.java)
34+
private const val MAX_WINDOW_SIZE = 0xFFFFFFFFL
3335
}
3436

3537
private var _isOpen = true
@@ -55,6 +57,12 @@ internal class AgentChannel(
5557
}
5658

5759
fun onWindowAdjust(bytesToAdd: Long) {
60+
if (bytesToAdd <= 0) {
61+
throw SshException("Invalid window adjust: bytesToAdd must be positive, got $bytesToAdd")
62+
}
63+
if (remoteWindowSize + bytesToAdd > MAX_WINDOW_SIZE) {
64+
throw SshException("Channel window overflow: current=$remoteWindowSize, adding=$bytesToAdd exceeds max $MAX_WINDOW_SIZE")
65+
}
5866
remoteWindowSize += bytesToAdd
5967
logger.debug("Agent channel window adjust +$bytesToAdd, remote window now $remoteWindowSize")
6068
if (remoteWindowSize > 0) {

sshlib/src/main/kotlin/org/connectbot/sshlib/client/ForwardingChannel.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* ConnectBot SSH Library
3-
* Copyright 2025 Kenny Root
3+
* Copyright 2025-2026 Kenny Root
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@ package org.connectbot.sshlib.client
1919

2020
import kotlinx.coroutines.channels.Channel
2121
import kotlinx.coroutines.channels.ReceiveChannel
22+
import org.connectbot.sshlib.SshException
2223
import org.slf4j.LoggerFactory
2324

2425
internal class ForwardingChannel(
@@ -32,6 +33,7 @@ internal class ForwardingChannel(
3233
companion object {
3334
private val logger = LoggerFactory.getLogger(ForwardingChannel::class.java)
3435
private const val WINDOW_ADJUST_THRESHOLD = 64 * 1024
36+
private const val MAX_WINDOW_SIZE = 0xFFFFFFFFL
3537
}
3638

3739
private var _isOpen = true
@@ -57,6 +59,12 @@ internal class ForwardingChannel(
5759
}
5860

5961
internal fun onWindowAdjust(bytesToAdd: Long) {
62+
if (bytesToAdd <= 0) {
63+
throw SshException("Invalid window adjust: bytesToAdd must be positive, got $bytesToAdd")
64+
}
65+
if (remoteWindowSize + bytesToAdd > MAX_WINDOW_SIZE) {
66+
throw SshException("Channel window overflow: current=$remoteWindowSize, adding=$bytesToAdd exceeds max $MAX_WINDOW_SIZE")
67+
}
6068
remoteWindowSize += bytesToAdd
6169
logger.debug("Forwarding channel window adjust +$bytesToAdd, remote window now $remoteWindowSize")
6270
if (remoteWindowSize > 0) {

sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* ConnectBot SSH Library
3-
* Copyright 2025 Kenny Root
3+
* Copyright 2025-2026 Kenny Root
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@ import kotlinx.coroutines.delay
2525
import kotlinx.coroutines.launch
2626
import kotlinx.coroutines.sync.Mutex
2727
import kotlinx.coroutines.sync.withLock
28+
import org.connectbot.sshlib.SshException
2829
import org.connectbot.sshlib.SshSession
2930
import org.connectbot.sshlib.protocol.ByteString
3031
import org.connectbot.sshlib.protocol.ChannelRequestExec
@@ -52,6 +53,7 @@ class SessionChannel internal constructor(
5253
companion object {
5354
private val logger = LoggerFactory.getLogger(SessionChannel::class.java)
5455
private const val WINDOW_ADJUST_THRESHOLD = 16 * 1024
56+
private const val MAX_WINDOW_SIZE = 0xFFFFFFFFL
5557
}
5658

5759
private var _isOpen = true
@@ -109,6 +111,12 @@ class SessionChannel internal constructor(
109111
}
110112

111113
internal fun onWindowAdjust(bytesToAdd: Long) {
114+
if (bytesToAdd <= 0) {
115+
throw SshException("Invalid window adjust: bytesToAdd must be positive, got $bytesToAdd")
116+
}
117+
if (remoteWindowSize + bytesToAdd > MAX_WINDOW_SIZE) {
118+
throw SshException("Channel window overflow: current=$remoteWindowSize, adding=$bytesToAdd exceeds max $MAX_WINDOW_SIZE")
119+
}
112120
remoteWindowSize += bytesToAdd
113121
logger.debug("Window adjust +$bytesToAdd, remote window now $remoteWindowSize")
114122
if (remoteWindowSize > 0) {

sshlib/src/test/kotlin/org/connectbot/sshlib/client/SessionChannelTest.kt

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* ConnectBot SSH Library
3-
* Copyright 2025 Kenny Root
3+
* Copyright 2025-2026 Kenny Root
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -25,11 +25,13 @@ import kotlinx.coroutines.CoroutineScope
2525
import kotlinx.coroutines.ExperimentalCoroutinesApi
2626
import kotlinx.coroutines.test.UnconfinedTestDispatcher
2727
import kotlinx.coroutines.test.runTest
28+
import org.connectbot.sshlib.SshException
2829
import org.junit.jupiter.api.Assertions.assertArrayEquals
2930
import org.junit.jupiter.api.Assertions.assertEquals
3031
import org.junit.jupiter.api.Assertions.assertFalse
3132
import org.junit.jupiter.api.Assertions.assertTrue
3233
import org.junit.jupiter.api.Test
34+
import kotlin.test.assertFailsWith
3335

3436
@OptIn(ExperimentalCoroutinesApi::class)
3537
class SessionChannelTest {
@@ -157,4 +159,45 @@ class SessionChannelTest {
157159

158160
coVerify(exactly = 1) { conn.sendChannelClose(1) }
159161
}
162+
163+
@Test
164+
fun `onWindowAdjust throws when remote window would exceed max uint32`() = runTest {
165+
// Start with a large initial window and try to push it over 2^32-1
166+
val (channel, _) = createChannel(
167+
connection = mockk(relaxed = true),
168+
initialWindowSize = 64 * 1024,
169+
)
170+
// channel starts with remoteWindowSizeInitial = 64 * 1024; add enough to overflow uint32
171+
val overflow = (0xFFFFFFFFL - 64 * 1024L) + 1L
172+
assertFailsWith<SshException> {
173+
channel.onWindowAdjust(overflow)
174+
}
175+
}
176+
177+
@Test
178+
fun `onWindowAdjust accepts legitimate adjustment within uint32 range`() = runTest {
179+
val (channel, _) = createChannel(
180+
connection = mockk(relaxed = true),
181+
initialWindowSize = 64 * 1024,
182+
)
183+
// Valid: bring window up to exactly 2^32-1
184+
val maxAdd = 0xFFFFFFFFL - 64 * 1024L
185+
channel.onWindowAdjust(maxAdd) // should not throw
186+
}
187+
188+
@Test
189+
fun `onWindowAdjust rejects zero adjustment`() = runTest {
190+
val (channel, _) = createChannel(connection = mockk(relaxed = true))
191+
assertFailsWith<SshException> {
192+
channel.onWindowAdjust(0L)
193+
}
194+
}
195+
196+
@Test
197+
fun `onWindowAdjust rejects negative adjustment`() = runTest {
198+
val (channel, _) = createChannel(connection = mockk(relaxed = true))
199+
assertFailsWith<SshException> {
200+
channel.onWindowAdjust(-1L)
201+
}
202+
}
160203
}

0 commit comments

Comments
 (0)