From 3a6193cbbf15b2118c2ff05161fafe99b792016d Mon Sep 17 00:00:00 2001 From: Jeff <11721615+jeffellin@users.noreply.github.com> Date: Tue, 17 Feb 2026 12:20:03 -0500 Subject: [PATCH] Fix parser corruption on unknown channel request types Motivation: NIOSSH suffers from critical parser state corruption when receiving SSH channel requests with unknown request type strings. The parser fails to consume payload bytes, leaving unconsumed data that corrupts all subsequent packet reads, causing permanent connection failure. This blocks production usage with enterprise SSH servers, cloud providers, and security appliances that send vendor-specific channel request extensions using the RFC 4250 name@domain format. Modifications: - readChannelRequestMessage(): Consume remaining bytes with moveReaderIndex(to: writerIndex) before setting type = .unknown - Add testUnknownChannelRequestWithPayload to verify bytes consumed - Add testParserContinuesAfterUnknownChannelRequest to verify parser state remains valid This matches the existing pattern in readGlobalRequestMessage() which correctly consumes payload bytes for unknown global requests. Result: NIOSSH can now handle SSH channel requests with unknown request types without connection corruption. Connections continue normally when encountering vendor-specific extensions, enabling interoperability with enterprise SSH servers, cloud providers, and security appliances. --- Sources/NIOSSH/SSHMessages.swift | 4 ++ Tests/NIOSSHTests/SSHMessagesTests.swift | 60 ++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/Sources/NIOSSH/SSHMessages.swift b/Sources/NIOSSH/SSHMessages.swift index a127d336..2a080cda 100644 --- a/Sources/NIOSSH/SSHMessages.swift +++ b/Sources/NIOSSH/SSHMessages.swift @@ -1128,6 +1128,10 @@ extension ByteBuffer { } type = .signal(signalName) default: + // Unknown channel request type - consume remaining bytes to prevent + // parser state corruption. Without this, leftover bytes cause + // invalidPacketFormat errors that corrupt the encryption state. + self.moveReaderIndex(to: self.writerIndex) type = .unknown } diff --git a/Tests/NIOSSHTests/SSHMessagesTests.swift b/Tests/NIOSSHTests/SSHMessagesTests.swift index 442ab36a..6a1864a0 100644 --- a/Tests/NIOSSHTests/SSHMessagesTests.swift +++ b/Tests/NIOSSHTests/SSHMessagesTests.swift @@ -797,4 +797,64 @@ final class SSHMessagesTests: XCTestCase { try self.assertCorrectlyManagesPartialRead(message) } + + func testUnknownChannelRequestWithPayload() throws { + // This test verifies that unknown channel request types with payloads + // are handled correctly without corrupting the parser state. + var buffer = ByteBufferAllocator().buffer(capacity: 100) + + // Build a channel request with unknown type and payload + buffer.writeInteger(SSHMessage.ChannelRequestMessage.id) + buffer.writeInteger(UInt32(0)) // Recipient channel + buffer.writeSSHString("custom-extension@example.com".utf8) // Unknown type + buffer.writeSSHBoolean(false) // Want reply + buffer.writeSSHString("extension-payload-data".utf8) // Payload + + let message = try buffer.readSSHMessage() + + guard case .some(.channelRequest(let request)) = message else { + XCTFail("Expected channelRequest message") + return + } + + XCTAssertEqual(request.recipientChannel, 0) + XCTAssertEqual(request.type, .unknown) + XCTAssertFalse(request.wantReply) + + // Verify buffer is fully consumed (no leftover bytes that would corrupt next read) + XCTAssertEqual(buffer.readableBytes, 0, "Parser should consume all bytes for unknown channel request") + } + + func testParserContinuesAfterUnknownChannelRequest() throws { + // This test verifies that the parser can continue reading valid messages + // after encountering an unknown channel request type. + var buffer = ByteBufferAllocator().buffer(capacity: 200) + + // Write unknown channel request + buffer.writeInteger(SSHMessage.ChannelRequestMessage.id) + buffer.writeInteger(UInt32(0)) + buffer.writeSSHString("unknown-request@example.com".utf8) + buffer.writeSSHBoolean(false) + buffer.writeSSHString("payload-data".utf8) + + // Read first message + let message1 = try buffer.readSSHMessage() + XCTAssertNotNil(message1) + guard case .some(.channelRequest(let request)) = message1 else { + XCTFail("Expected channelRequest") + return + } + XCTAssertEqual(request.type, .unknown) + + // Write a known message after + buffer.writeInteger(SSHMessage.ChannelSuccessMessage.id) + buffer.writeInteger(UInt32(0)) + + // Verify parser can continue + let message2 = try buffer.readSSHMessage() + guard case .some(.channelSuccess) = message2 else { + XCTFail("Expected channelSuccess after unknown channel request - parser state may be corrupted") + return + } + } }