From: Victor Julien <victor@inliniac.net>
Date: Fri, 3 Apr 2020 14:31:00 +0000 (+0200)
Subject: ssl: copy data using a safe memcpy wrapper
X-Git-Tag: suricata-6.0.0-beta1~448
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d1ada2e13c207e0937f8a4818d5731d319f5fa07;p=thirdparty%2Fsuricata.git

ssl: copy data using a safe memcpy wrapper

To avoid future memcpy issues introduce a wrapper and check the
result of it.

When compiled with --enable-debug-validation the wrapper will abort if
the input is wrong.
---

diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c
index 29ff56d6cc..f0b474e5ac 100644
--- a/src/app-layer-ssl.c
+++ b/src/app-layer-ssl.c
@@ -52,6 +52,7 @@
 #include "util-ja3.h"
 #include "flow-util.h"
 #include "flow-private.h"
+#include "util-validate.h"
 
 SCEnumCharMap tls_decoder_event_table[ ] = {
     /* TLS protocol messages */
@@ -175,6 +176,26 @@ SslConfig ssl_config;
 
 #define HAS_SPACE(n) ((uint64_t)(input - initial_input) + (uint64_t)(n) <= (uint64_t)(input_len))
 
+static inline int SafeMemcpy(void *dst, size_t dst_offset, size_t dst_size,
+        const void *src, size_t src_offset, size_t src_size, size_t src_tocopy) WARN_UNUSED;
+
+static inline int SafeMemcpy(void *dst, size_t dst_offset, size_t dst_size,
+        const void *src, size_t src_offset, size_t src_size, size_t src_tocopy)
+{
+    DEBUG_VALIDATE_BUG_ON(dst_offset >= dst_size);
+    DEBUG_VALIDATE_BUG_ON(src_offset >= src_size);
+    DEBUG_VALIDATE_BUG_ON(src_tocopy > (src_size - src_offset));
+    DEBUG_VALIDATE_BUG_ON(src_tocopy > (dst_size - dst_offset));
+
+    if (dst_offset < dst_size && src_offset < src_size &&
+        src_tocopy <= (src_size - src_offset) &&
+        src_tocopy <= (dst_size - dst_offset)) {
+        memcpy(dst + dst_offset, src + src_offset, src_tocopy);
+        return 0;
+    }
+    return -1;
+}
+
 static void SSLParserReset(SSLState *ssl_state)
 {
     ssl_state->curr_connp->bytes_processed = 0;
@@ -705,7 +726,10 @@ static inline int TLSDecodeHSHelloSessionID(SSLState *ssl_state,
             return -1;
         }
 
-        memcpy(ssl_state->curr_connp->session_id, input, session_id_length);
+        if (SafeMemcpy(ssl_state->curr_connp->session_id, 0, session_id_length,
+                    input, 0, input_len, session_id_length) != 0) {
+            return -1;
+        }
         ssl_state->curr_connp->session_id_length = session_id_length;
 
         if ((ssl_state->current_flags & SSL_AL_FLAG_STATE_SERVER_HELLO) &&
@@ -902,7 +926,13 @@ static inline int TLSDecodeHSHelloExtensionSni(SSLState *ssl_state,
     if (unlikely(ssl_state->curr_connp->sni == NULL))
         return -1;
 
-    memcpy(ssl_state->curr_connp->sni, input, sni_strlen - 1);
+    const size_t consumed = input - initial_input;
+    if (SafeMemcpy(ssl_state->curr_connp->sni, 0, sni_strlen,
+                initial_input, consumed, input_len, sni_len) != 0) {
+        SCFree(ssl_state->curr_connp->sni);
+        ssl_state->curr_connp->sni = NULL;
+        return -1;
+    }
     ssl_state->curr_connp->sni[sni_strlen-1] = 0;
 
     input += sni_len;
@@ -1459,8 +1489,12 @@ static int SSLv3ParseHandshakeType(SSLState *ssl_state, const uint8_t *input,
                 write_len = input_len;
             }
 
-            memcpy(ssl_state->curr_connp->trec +
-                    ssl_state->curr_connp->trec_pos, initial_input, write_len);
+            if (SafeMemcpy(ssl_state->curr_connp->trec,
+                        ssl_state->curr_connp->trec_pos,
+                        ssl_state->curr_connp->trec_len,
+                        initial_input, 0, input_len, write_len) != 0) {
+                return -1;
+            }
             ssl_state->curr_connp->trec_pos += write_len;
 
             rc = TlsDecodeHSCertificate(ssl_state, ssl_state->curr_connp->trec,