From 2552b2e8c9f73b5cdef8842e4e3701bac19c12c6 Mon Sep 17 00:00:00 2001 From: Adi Roiban <adi.roiban@chevah.com> Date: Mon, 24 Jan 2022 19:09:04 +0000 Subject: [PATCH 3/4] Fix CVE-2022-21716 Initial fix for Twisted version string DoS. (cherry picked from commit de90dfe1519e996dd150de751c670f8e03daa089) Update after review. (cherry picked from commit 9b98116372f5d211ddb9f68916d4ae73bf3c8da7) Fix typo. (cherry picked from commit a4523b444760f07e609636264a61a2a07ca0bde5) --- src/twisted/conch/ssh/transport.py | 9 +++++++++ src/twisted/conch/test/test_transport.py | 21 +++++++++++++++++++++ src/twisted/newsfragments/10284.bugfix | 2 ++ 3 files changed, 32 insertions(+) create mode 100644 src/twisted/newsfragments/10284.bugfix diff --git a/src/twisted/conch/ssh/transport.py b/src/twisted/conch/ssh/transport.py index bd76b0a845..6d02df52dd 100644 --- a/src/twisted/conch/ssh/transport.py +++ b/src/twisted/conch/ssh/transport.py @@ -677,6 +677,15 @@ class SSHTransportBase(protocol.Protocol): """ self.buf = self.buf + data if not self.gotVersion: + + if len(self.buf) > 4096: + self.sendDisconnect( + DISCONNECT_CONNECTION_LOST, + b"Peer version string longer than 4KB. " + b"Preventing a denial of service attack.", + ) + return + if self.buf.find(b'\n', self.buf.find(b'SSH-')) == -1: return diff --git a/src/twisted/conch/test/test_transport.py b/src/twisted/conch/test/test_transport.py index 98a3515a75..449dd3f8df 100644 --- a/src/twisted/conch/test/test_transport.py +++ b/src/twisted/conch/test/test_transport.py @@ -522,6 +522,27 @@ class BaseSSHTransportTests(BaseSSHTransportBaseCase, TransportTestCase): r')*$') self.assertRegex(softwareVersion, softwareVersionRegex) + def test_dataReceiveVersionNotSentMemoryDOS(self): + """ + When the peer is not sending its SSH version but keeps sending data, + the connection is disconnected after 4KB to prevent buffering too + much and running our of memory. + """ + sut = MockTransportBase() + sut.makeConnection(self.transport) + + # Data can be received over multiple chunks. + sut.dataReceived(b"SSH-2-Server-Identifier") + sut.dataReceived(b"1234567890" * 406) + sut.dataReceived(b"1235678") + self.assertFalse(self.transport.disconnecting) + + # Here we are going over the limit. + sut.dataReceived(b"1234567") + # Once a lot of data is received without an SSH version string, + # the transport is disconnected. + self.assertTrue(self.transport.disconnecting) + self.assertIn(b"Preventing a denial of service attack", self.transport.value()) def test_sendPacketPlain(self): """ diff --git a/src/twisted/newsfragments/10284.bugfix b/src/twisted/newsfragments/10284.bugfix new file mode 100644 index 0000000000..b2316f1e68 --- /dev/null +++ b/src/twisted/newsfragments/10284.bugfix @@ -0,0 +1,2 @@ +twisted.conch.ssh.transport.SSHTransportBase now disconnects the remote peer if the +SSH version string is not sent in the first 4096 bytes. -- 2.39.2