Revert "Fix out-of-bounds read when TLS msg is split up into multiple…

… chunks" This reverts commit 18f993c.
tmshort · Dec 11, 2020 · 3e7fb5a · 3e7fb5a
1 parent 4a1a98e
commit 3e7fb5a
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 76 deletions.
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
@@ -1410,8 +1410,6 @@ struct ssl_st {
     OSSL_ENCRYPTION_LEVEL quic_write_level;
     QUIC_DATA *quic_input_data_head;
     QUIC_DATA *quic_input_data_tail;
-    uint8_t quic_msg_hd[SSL3_HM_HEADER_LENGTH];
-    size_t quic_msg_hd_offset;
     const SSL_QUIC_METHOD *quic_method;
 #endif
     /*

diff --git a/ssl/ssl_quic.c b/ssl/ssl_quic.c
@@ -93,7 +93,6 @@ int SSL_provide_quic_data(SSL *ssl, OSSL_ENCRYPTION_LEVEL level,
 {
     size_t l;
     uint8_t mt;
-    QUIC_DATA *qd;
 
     if (!SSL_IS_QUIC(ssl)) {
         SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
@@ -107,65 +106,35 @@ int SSL_provide_quic_data(SSL *ssl, OSSL_ENCRYPTION_LEVEL level,
         return 0;
     }
 
-    if (len == 0) {
-      return 1;
-    }
-
-    /* Check for an incomplete block */
-    qd = ssl->quic_input_data_tail;
-    if (qd != NULL) {
-        l = qd->length - qd->offset;
-        if (l != 0) {
-            /* we still need to copy `l` bytes into the last data block */
-            if (l > len)
-                l = len;
-            memcpy((char *)(qd + 1) + qd->offset, data, l);
-            qd->offset += l;
-            len -= l;
-            data += l;
-        }
-    }
-
-    /* Split the QUIC messages up, if necessary */
+    /* Split on handshake message boundaries, if necessary */
     while (len > 0) {
+        QUIC_DATA *qd;
         const uint8_t *p;
-        uint8_t *dst;
-
-        if (ssl->quic_msg_hd_offset != 0) {
-            /* If we have already buffered premature message header,
-               try to add new data to it to form complete message
-               header. */
-            size_t nread =
-                SSL3_HM_HEADER_LENGTH - ssl->quic_msg_hd_offset;
-
-            if (len < nread)
-                nread = len;
-            memcpy(ssl->quic_msg_hd + ssl->quic_msg_hd_offset, data, nread);
-            ssl->quic_msg_hd_offset += nread;
-
-            if (ssl->quic_msg_hd_offset < SSL3_HM_HEADER_LENGTH) {
-                /* We still have premature message header. */
-                break;
+
+        /* Check for an incomplete block */
+        qd = ssl->quic_input_data_tail;
+        if (qd != NULL) {
+            l = qd->length - qd->offset;
+            if (l != 0) {
+                /* we still need to copy `l` bytes into the last data block */
+                if (l > len)
+                    l = len;
+                memcpy((char*)(qd+1) + qd->offset, data, l);
+                qd->offset += l;
+                len -= l;
+                data += l;
+                continue;
             }
-            data += nread;
-            len -= nread;
-            /* TLS Handshake message header has 1-byte type and 3-byte length */
-            mt = *ssl->quic_msg_hd;
-            p = ssl->quic_msg_hd + 1;
-            n2l3(p, l);
-        } else if (len < SSL3_HM_HEADER_LENGTH) {
-            /* We don't get complete message header.  Just buffer the
-               received data and wait for the next data to arrive. */
-            memcpy(ssl->quic_msg_hd, data, len);
-            ssl->quic_msg_hd_offset += len;
-            break;
-        } else {
-            /* We have complete message header in data. */
-            /* TLS Handshake message header has 1-byte type and 3-byte length */
-            mt = *data;
-            p = data + 1;
-            n2l3(p, l);
         }
+
+        if (len < SSL3_HM_HEADER_LENGTH) {
+            SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, SSL_R_BAD_LENGTH);
+            return 0;
+        }
+        /* TLS Handshake message header has 1-byte type and 3-byte length */
+        mt = *data;
+        p = data + 1;
+        n2l3(p, l);
         l += SSL3_HM_HEADER_LENGTH;
         if (mt == SSL3_MT_KEY_UPDATE) {
             SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, SSL_R_UNEXPECTED_MESSAGE);
@@ -181,23 +150,12 @@ int SSL_provide_quic_data(SSL *ssl, OSSL_ENCRYPTION_LEVEL level,
         qd->next = NULL;
         qd->length = l;
         qd->level = level;
+        /* partial data received? */
+        if (l > len)
+            l = len;
+        qd->offset = l;
 
-        dst = (uint8_t *)(qd + 1);
-        if (ssl->quic_msg_hd_offset) {
-            memcpy(dst, ssl->quic_msg_hd, ssl->quic_msg_hd_offset);
-            dst += ssl->quic_msg_hd_offset;
-            l -= SSL3_HM_HEADER_LENGTH;
-            if (l > len)
-                l = len;
-            qd->offset = SSL3_HM_HEADER_LENGTH + l;
-            memcpy(dst, data, l);
-        } else {
-            /* partial data received? */
-            if (l > len)
-                l = len;
-            qd->offset = l;
-            memcpy(dst, data, l);
-        }
+        memcpy((void*)(qd + 1), data, l);
         if (ssl->quic_input_data_tail != NULL)
             ssl->quic_input_data_tail->next = qd;
         else
@@ -206,8 +164,6 @@ int SSL_provide_quic_data(SSL *ssl, OSSL_ENCRYPTION_LEVEL level,
 
         data += l;
         len -= l;
-
-        ssl->quic_msg_hd_offset = 0;
     }
 
     return 1;