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 84a3f4a309364f82d2ce160a0aa15adc458a6c64..c29ecc7a573ac0a123ad17ca15cca630c4eda68c 100644 (file)
--- 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 345f53d91b6554c2e572e5a563714600e76fc68f..17eb8b0402a85eb5eded6250e706e27ae9a795fe 100644 (file)
--- 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 8422d814a1be02cec422f0a269fe21978c82a709..0845e62d79a1bfca5d3855b95ccaa20e5bc30a84 100644 (file)
--- 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.