From be728cba83fda33925d1c0f901aaa1054c88c14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net> Date: Thu, 22 Feb 2024 22:09:46 -0500 Subject: [PATCH] Read SSH packet headers into a fixed buffer in the session info Previously, it was read into a local variable, and in the case of a partial read, everything would go to hell and SSH would hang. "Luckily" this was very hard to trigger. --- 3rdp/build/GNUmakefile | 3 +- 3rdp/build/cl-fix-ssh-header-read.patch | 87 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 3rdp/build/cl-fix-ssh-header-read.patch diff --git a/3rdp/build/GNUmakefile b/3rdp/build/GNUmakefile index cd73d947ab..efe18687c3 100644 --- a/3rdp/build/GNUmakefile +++ b/3rdp/build/GNUmakefile @@ -90,7 +90,7 @@ $(CRYPT_SRC): | $(3RDPSRCDIR) $(CRYPT_IDIR): | $(3RDPODIR) $(QUIET)$(IFNOTEXIST) mkdir $(CRYPT_IDIR) -$(CRYPTLIB_BUILD): $(3RDP_ROOT)/dist/cryptlib.zip $(3RDP_ROOT)/build/cl-fix-test-select.patch $(3RDP_ROOT)/build/cl-terminal-params.patch $(3RDP_ROOT)/build/cl-mingw32-static.patch $(3RDP_ROOT)/build/cl-ranlib.patch $(3RDP_ROOT)/build/cl-win32-noasm.patch $(3RDP_ROOT)/build/cl-zz-country.patch $(3RDP_ROOT)/build/cl-algorithms.patch $(3RDP_ROOT)/build/cl-allow-duplicate-ext.patch $(3RDP_ROOT)/build/cl-macosx-minver.patch $(3RDP_ROOT)/build/cl-posix-me-gently.patch $(3RDP_ROOT)/build/cl-PAM-noprompts.patch $(3RDP_ROOT)/build/cl-zlib.patch $(3RDP_ROOT)/build/cl-Dynamic-linked-static-lib.patch $(3RDP_ROOT)/build/cl-SSL-fix.patch $(3RDP_ROOT)/build/cl-bigger-maxattribute.patch $(3RDP_ROOT)/build/cl-endian.patch $(3RDP_ROOT)/build/cl-vcxproj.patch $(3RDP_ROOT)/build/cl-mingw-vcver.patch $(3RDP_ROOT)/build/cl-win32-build-fix.patch $(3RDP_ROOT)/build/cl-no-odbc.patch $(3RDP_ROOT)/build/cl-noasm-defines.patch $(3RDP_ROOT)/build/cl-bn-noasm64-fix.patch $(3RDP_ROOT)/build/cl-prefer-ECC.patch $(3RDP_ROOT)/build/cl-prefer-ECC-harder.patch $(3RDP_ROOT)/build/cl-clear-GCM-flag.patch $(3RDP_ROOT)/build/cl-use-ssh-ctr.patch $(3RDP_ROOT)/build/cl-ssl-suite-blocksizes.patch $(3RDP_ROOT)/build/cl-no-tpm.patch $(3RDP_ROOT)/build/cl-no-via-aes.patch $(3RDP_ROOT)/build/cl-fix-ssh-ecc-ephemeral.patch $(3RDP_ROOT)/build/cl-just-use-cc.patch $(3RDP_ROOT)/build/cl-no-safe-stack.patch $(3RDP_ROOT)/build/cl-allow-pkcs12.patch $(3RDP_ROOT)/build/cl-openbsd-threads.patch $(3RDP_ROOT)/build/cl-allow-none-auth.patch $(3RDP_ROOT)/build/cl-mingw-add-m32.patch $(3RDP_ROOT)/build/cl-poll-not-select.patch $(3RDP_ROOT)/build/cl-good-sockets.patch $(3RDP_ROOT)/build/cl-moar-objects.patch $(3RDP_ROOT)/build/cl-server-term-support.patch $(3RDP_ROOT)/build/cl-add-pubkey-attribute.patch $(3RDP_ROOT)/build/cl-allow-ssh-auth-retries.patch $(3RDP_ROOT)/build/cl-fix-ssh-channel-close.patch $(3RDP_ROOT)/build/cl-vt-lt-2005-always-defined.patch $(3RDP_ROOT)/build/cl-no-pie.patch $(3RDP_ROOT)/build/cl-no-testobjs.patch $(3RDP_ROOT)/build/cl-win32-lean-and-mean.patch $(3RDP_ROOT)/build/cl-thats-not-asm.patch $(3RDP_ROOT)/build/cl-make-channels-work.patch $(3RDP_ROOT)/build/cl-allow-ssh-2.0-go.patch $(3RDP_ROOT)/build/cl-read-timeout-every-time.patch $(3RDP_ROOT)/build/cl-allow-servercheck-pubkeys.patch $(3RDP_ROOT)/build/cl-pass-after-pubkey.patch $(3RDP_ROOT)/build/cl-ssh-list-ctr-modes.patch $(3RDP_ROOT)/build/cl-double-delete-fine-on-close.patch $(3RDP_ROOT)/build/cl-handle-unsupported-pubkey.patch $(3RDP_ROOT)/build/cl-add-patches-info.patch $(3RDP_ROOT)/build/cl-netbsd-hmac-symbol.patch $(3RDP_ROOT)/build/cl-netbsd-no-getfsstat.patch GNUmakefile $(3RDP_ROOT)/build/cl-remove-march.patch $(3RDP_ROOT)/build/cl-add-win64.patch $(3RDP_ROOT)/build/cl-fix-mb-w-conv-warnings.patch | $(CRYPT_SRC) $(CRYPT_IDIR) +$(CRYPTLIB_BUILD): $(3RDP_ROOT)/dist/cryptlib.zip $(3RDP_ROOT)/build/cl-fix-test-select.patch $(3RDP_ROOT)/build/cl-terminal-params.patch $(3RDP_ROOT)/build/cl-mingw32-static.patch $(3RDP_ROOT)/build/cl-ranlib.patch $(3RDP_ROOT)/build/cl-win32-noasm.patch $(3RDP_ROOT)/build/cl-zz-country.patch $(3RDP_ROOT)/build/cl-algorithms.patch $(3RDP_ROOT)/build/cl-allow-duplicate-ext.patch $(3RDP_ROOT)/build/cl-macosx-minver.patch $(3RDP_ROOT)/build/cl-posix-me-gently.patch $(3RDP_ROOT)/build/cl-PAM-noprompts.patch $(3RDP_ROOT)/build/cl-zlib.patch $(3RDP_ROOT)/build/cl-Dynamic-linked-static-lib.patch $(3RDP_ROOT)/build/cl-SSL-fix.patch $(3RDP_ROOT)/build/cl-bigger-maxattribute.patch $(3RDP_ROOT)/build/cl-endian.patch $(3RDP_ROOT)/build/cl-vcxproj.patch $(3RDP_ROOT)/build/cl-mingw-vcver.patch $(3RDP_ROOT)/build/cl-win32-build-fix.patch $(3RDP_ROOT)/build/cl-no-odbc.patch $(3RDP_ROOT)/build/cl-noasm-defines.patch $(3RDP_ROOT)/build/cl-bn-noasm64-fix.patch $(3RDP_ROOT)/build/cl-prefer-ECC.patch $(3RDP_ROOT)/build/cl-prefer-ECC-harder.patch $(3RDP_ROOT)/build/cl-clear-GCM-flag.patch $(3RDP_ROOT)/build/cl-use-ssh-ctr.patch $(3RDP_ROOT)/build/cl-ssl-suite-blocksizes.patch $(3RDP_ROOT)/build/cl-no-tpm.patch $(3RDP_ROOT)/build/cl-no-via-aes.patch $(3RDP_ROOT)/build/cl-fix-ssh-ecc-ephemeral.patch $(3RDP_ROOT)/build/cl-just-use-cc.patch $(3RDP_ROOT)/build/cl-no-safe-stack.patch $(3RDP_ROOT)/build/cl-allow-pkcs12.patch $(3RDP_ROOT)/build/cl-openbsd-threads.patch $(3RDP_ROOT)/build/cl-allow-none-auth.patch $(3RDP_ROOT)/build/cl-mingw-add-m32.patch $(3RDP_ROOT)/build/cl-poll-not-select.patch $(3RDP_ROOT)/build/cl-good-sockets.patch $(3RDP_ROOT)/build/cl-moar-objects.patch $(3RDP_ROOT)/build/cl-server-term-support.patch $(3RDP_ROOT)/build/cl-add-pubkey-attribute.patch $(3RDP_ROOT)/build/cl-allow-ssh-auth-retries.patch $(3RDP_ROOT)/build/cl-fix-ssh-channel-close.patch $(3RDP_ROOT)/build/cl-vt-lt-2005-always-defined.patch $(3RDP_ROOT)/build/cl-no-pie.patch $(3RDP_ROOT)/build/cl-no-testobjs.patch $(3RDP_ROOT)/build/cl-win32-lean-and-mean.patch $(3RDP_ROOT)/build/cl-thats-not-asm.patch $(3RDP_ROOT)/build/cl-make-channels-work.patch $(3RDP_ROOT)/build/cl-allow-ssh-2.0-go.patch $(3RDP_ROOT)/build/cl-read-timeout-every-time.patch $(3RDP_ROOT)/build/cl-allow-servercheck-pubkeys.patch $(3RDP_ROOT)/build/cl-pass-after-pubkey.patch $(3RDP_ROOT)/build/cl-ssh-list-ctr-modes.patch $(3RDP_ROOT)/build/cl-double-delete-fine-on-close.patch $(3RDP_ROOT)/build/cl-handle-unsupported-pubkey.patch $(3RDP_ROOT)/build/cl-add-patches-info.patch $(3RDP_ROOT)/build/cl-netbsd-hmac-symbol.patch $(3RDP_ROOT)/build/cl-netbsd-no-getfsstat.patch GNUmakefile $(3RDP_ROOT)/build/cl-remove-march.patch $(3RDP_ROOT)/build/cl-add-win64.patch $(3RDP_ROOT)/build/cl-fix-mb-w-conv-warnings.patch $(3RDP_ROOT)/build/cl-fix-ssh-header-read.patch | $(CRYPT_SRC) $(CRYPT_IDIR) @echo Creating $@ ... $(QUIET)-rm -rf $(CRYPT_SRC)/* $(QUIET)unzip -oa $(3RDPDISTDIR)/cryptlib.zip -d $(CRYPT_SRC) @@ -157,6 +157,7 @@ $(CRYPTLIB_BUILD): $(3RDP_ROOT)/dist/cryptlib.zip $(3RDP_ROOT)/build/cl-fix-test $(QUIET)patch -b -p0 -d $(CRYPT_SRC) < cl-netbsd-no-getfsstat.patch $(QUIET)patch -b -p0 -d $(CRYPT_SRC) < cl-add-win64.patch $(QUIET)patch -b -p0 -d $(CRYPT_SRC) < cl-fix-mb-w-conv-warnings.patch + $(QUIET)patch -b -p0 -d $(CRYPT_SRC) < cl-fix-ssh-header-read.patch $(QUIET)perl -pi.bak -e 's/^(#define CRYPTLIB_VERSION.*)$$/"$$1\n#define CRYPTLIB_PATCHES \"" . (chomp($$val = `cat cl-*.patch | if (which md5sum > \/dev\/null 2>&1); then md5sum; else md5; fi`), $$val) . "\""/e' $(CRYPT_SRC)/cryptlib.h ifeq ($(os),win32) $(QUIET)cd $(CRYPT_SRC) && env - PATH="$(PATH)" CC="$(CC)" AR="$(AR)" RANLIB="$(RANLIB)" make directories diff --git a/3rdp/build/cl-fix-ssh-header-read.patch b/3rdp/build/cl-fix-ssh-header-read.patch new file mode 100644 index 0000000000..358bf57870 --- /dev/null +++ b/3rdp/build/cl-fix-ssh-header-read.patch @@ -0,0 +1,87 @@ +--- session/session.h.orig 2024-02-22 21:46:07.919870000 -0500 ++++ session/session.h 2024-02-22 21:54:28.395535000 -0500 +@@ -370,6 +370,12 @@ + BUFFER_FIXED( KEYID_SIZE ) \ + BYTE authUserNameHash[ KEYID_SIZE + 8 ];/* Hashed userID */ + /* SSH_AUTHTYPE_TYPE */ int authType; /* Authentication method */ ++ ++ /* If a header is only partially read, cryptlib takes a shit because ++ it's in a local variable. Store it in the session data instead. */ ++ BUFFER_FIXED( 20 ) \ ++ BYTE headerBuffer[ 20 + 8 ]; ++ int headerBufferUsed; + } SSH_INFO; + #endif /* USE_SSH */ + +--- session/ssh2_rd.c.orig 2023-06-15 04:35:42.000000000 -0400 ++++ session/ssh2_rd.c 2024-02-22 21:57:48.805525000 -0500 +@@ -407,7 +407,6 @@ + const SSH_PROTOSTATE_TYPE protocolState ) + { + STREAM stream; +- BYTE headerBuffer[ LENGTH_SIZE + MIN_PACKET_SIZE + 8 ]; + const BOOLEAN isHandshake = \ + ( protocolState == SSH_PROTOSTATE_HANDSHAKE || \ + protocolState == SSH_PROTOSTATE_AUTH ) ? TRUE : FALSE; +@@ -460,13 +459,13 @@ + conditions due to buggy SSH implementations, to handle these we + check the return code as well as the returned data to see if we + need to process it specially */ +- status = readFixedHeaderAtomic( sessionInfoPtr, headerBuffer, ++ status = readFixedHeaderAtomic( sessionInfoPtr, sshInfo->headerBuffer, + headerByteCount ); + if( status == CRYPT_ERROR_READ || cryptStatusOK( status ) ) + { + const int localStatus = \ + checkHandshakePacketStatus( sessionInfoPtr, status, +- headerBuffer, headerByteCount, ++ sshInfo->headerBuffer, headerByteCount, + expectedType ); + if( cryptStatusError( localStatus ) ) + status = localStatus; +@@ -474,11 +473,12 @@ + } + else + { +- status = readFixedHeader( sessionInfoPtr, headerBuffer, +- headerByteCount ); ++ status = readFixedHeader( sessionInfoPtr, sshInfo->headerBuffer + sshInfo->headerBufferUsed, ++ headerByteCount - sshInfo->headerBufferUsed ); + } + if( cryptStatusError( status ) ) + return( status ); ++ sshInfo->headerBufferUsed = 0; + + /* If we're in the data-processing stage (i.e. it's a post-handshake + data packet read) exception conditions need to be handled specially +@@ -500,8 +500,8 @@ + of the message we decrypt */ + if( isSecureRead ) + { +- void *payloadPtr = useETM ? headerBuffer + LENGTH_SIZE : \ +- headerBuffer; ++ void *payloadPtr = useETM ? sshInfo->headerBuffer + LENGTH_SIZE : \ ++ sshInfo->headerBuffer; + + /* If we're using EtM then we have to preserve a copy of the + ciphertext so that we can MAC it later */ +@@ -541,7 +541,7 @@ + larger than the (remaining) data that we've already read. For + this case we need to check that the data payload is at least as + long as the minimum-length packet */ +- sMemConnect( &stream, headerBuffer, headerByteCount ); ++ sMemConnect( &stream, sshInfo->headerBuffer, headerByteCount ); + status = length = readUint32( &stream ); + static_assert( SSH_HEADER_REMAINDER_SIZE == MIN_PACKET_SIZE - \ + LENGTH_SIZE, \ +@@ -598,8 +598,8 @@ + from the stream above but have to manually extract it here */ + static_assert( LENGTH_SIZE + 1 + ID_SIZE <= MIN_PACKET_SIZE, + "Header length calculation" ); +- sshInfo->padLength = headerBuffer[ LENGTH_SIZE ]; +- sshInfo->packetType = headerBuffer[ LENGTH_SIZE + 1 ]; ++ sshInfo->padLength = sshInfo->headerBuffer[ LENGTH_SIZE ]; ++ sshInfo->packetType = sshInfo->headerBuffer[ LENGTH_SIZE + 1 ]; + if( sshInfo->padLength < SSH2_MIN_PADLENGTH_SIZE || \ + sshInfo->padLength > 255 ) + { -- GitLab