From: RAGHURAAM CONJEEVARAM UDAYANAN -X (rconjeev - XORIANT CORPORATION at Cisco) <rconjeev@cisco.com>
Date: Thu, 25 Jan 2024 15:08:14 +0000 (+0000)
Subject: Pull request #4140: ssl: heap overflow issue when processing handshake records
X-Git-Tag: 3.1.79.0~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ef70aa5986acae98498d140a0fa48220a5e4d359;p=thirdparty%2Fsnort3.git

Pull request #4140: ssl: heap overflow issue when processing handshake records

Merge in SNORT/snort3 from ~RCONJEEV/snort3:ssl_heap_overflow_issue to master

Squashed commit of the following:

commit 409d741819b112b39ebbb6dde991f6ec6c5ef762
Author: RAGHURAAM CONJEEVARAM UDAYANAN -X (rconjeev - XORIANT CORPORATION at Cisco) <rconjeev@cisco.com>
Date:   Mon Dec 11 17:50:26 2023 +0530

    ssl: heap overflow issue when processing handshake records
---

diff --git a/src/protocols/ssl.cc b/src/protocols/ssl.cc
index 84a3f4a30..c29ecc7a5 100644
--- a/src/protocols/ssl.cc
+++ b/src/protocols/ssl.cc
@@ -238,7 +238,7 @@ static uint32_t SSL_decode_handshake_v3(const uint8_t* pkt, int size,
 
 static uint32_t SSL_decode_v3(const uint8_t* pkt, int size, uint32_t pkt_flags,
     uint8_t* alert_flags, uint16_t* partial_rec_len, int max_hb_len, uint32_t* info_flags,
-    SSLV3ClientHelloData* client_hello_data, SSLV3ServerCertData* server_cert_data)
+    SSLV3ClientHelloData* client_hello_data, SSLV3ServerCertData* server_cert_data, uint32_t prev_flags)
 {
     uint32_t retval = 0;
     uint16_t hblen;
@@ -329,7 +329,7 @@ static uint32_t SSL_decode_v3(const uint8_t* pkt, int size, uint32_t pkt_flags,
         case SSL_HANDSHAKE_REC:
             /* If the CHANGE_CIPHER_FLAG is set, the following handshake
              * record should be encrypted */
-            if (!(retval & SSL_CHANGE_CIPHER_FLAG))
+            if (!(retval & SSL_CHANGE_CIPHER_FLAG) && !(prev_flags & SSL_CHANGE_CIPHER_FLAG))
             {
                 int hsize = size < (int)reclen ? size : (int)reclen;
                 retval |= SSL_decode_handshake_v3(pkt, hsize, retval, pkt_flags, client_hello_data, server_cert_data);
@@ -498,7 +498,7 @@ uint32_t SSL_decode(
          * indicate that it is truncated. */
         if (size == 5)
             return SSL_decode_v3(pkt, size, pkt_flags, alert_flags, partial_rec_len, max_hb_len, info_flags,
-                client_hello_data, server_cert_data);
+                client_hello_data, server_cert_data, prev_flags);
 
         /* At this point, 'size' has to be > 5 */
 
@@ -546,7 +546,7 @@ uint32_t SSL_decode(
     }
 
     return SSL_decode_v3(pkt, size, pkt_flags, alert_flags, partial_rec_len, max_hb_len, info_flags,
-        client_hello_data, server_cert_data);
+        client_hello_data, server_cert_data, prev_flags);
 }
 
 /* very simplistic - just enough to say this is binary data - the rules will make a final
diff --git a/src/protocols/ssl.h b/src/protocols/ssl.h
index 345f53d91..17eb8b040 100644
--- a/src/protocols/ssl.h
+++ b/src/protocols/ssl.h
@@ -289,6 +289,7 @@ struct ServiceSSLV3ExtensionServerName
 #define SSL_IS_CHELLO(x) ((x) & SSL_CLIENT_HELLO_FLAG)
 #define SSL_IS_SHELLO(x) ((x) & SSL_SERVER_HELLO_FLAG)
 #define SSL_IS_CKEYX(x) ((x) & SSL_CLIENT_KEYX_FLAG)
+#define SSL_IS_CHANGE_CIPHER(x) ((x) & SSL_CHANGE_CIPHER_FLAG)
 #define SSL_IS_APP(x) (((x) & SSL_SAPP_FLAG) || ((x) & SSL_CAPP_FLAG))
 #define SSL_IS_ALERT(x) ((x) & SSL_ALERT_FLAG)
 #define SSL_CLEAR_TEMPORARY_FLAGS(x) (x) &= ~SSL_STATEFLAGS
diff --git a/src/service_inspectors/ssl/ssl_inspector.cc b/src/service_inspectors/ssl/ssl_inspector.cc
index 8422d814a..0845e62d7 100644
--- a/src/service_inspectors/ssl/ssl_inspector.cc
+++ b/src/service_inspectors/ssl/ssl_inspector.cc
@@ -404,6 +404,14 @@ static void snort_ssl(SSL_PROTO_CONF* config, Packet* p)
     {
         sd->ssn_flags = SSLPP_process_app(config, sd->ssn_flags, new_flags, p);
     }
+    else if (SSL_IS_CHANGE_CIPHER(new_flags))
+    {
+        /* If the 'change cipher spec' and 'encrypted handshake message' flags come in separate subsequent packets,
+         * the encrypted handshake message is inspected, and attempts to process some random type and it fails.
+         * To avoid this situation, update the 'change cipher spec' flag in the session to skip processing
+         * the encrypted handshake message.*/
+        sd->ssn_flags |= SSL_CHANGE_CIPHER_FLAG;
+    }
     else
     {
         /* Different record type that we don't care about.