[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Mon Sep 26 10:05:58 UTC 2016


The branch master has been updated
       via  3133c2d3067c6add91cf370b0b8342d891b8e97a (commit)
       via  44f206aa9dfd4f226f17d9093732dbece5300aa6 (commit)
       via  0d698f6696e114a6e47f8b75ff88ec81f9e30175 (commit)
       via  f789b04f407c2003da62d2b91b587629f1a781d0 (commit)
       via  84d5549e692e63a16fa1b11603e4098fc31746e9 (commit)
      from  c536b6be1a72aefd632d5530106a67c516cb9f4b (commit)


- Log -----------------------------------------------------------------
commit 3133c2d3067c6add91cf370b0b8342d891b8e97a
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Sep 26 09:43:45 2016 +0100

    Updates CHANGES and NEWS for new release
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>

commit 44f206aa9dfd4f226f17d9093732dbece5300aa6
Author: Robert Swiecki <swiecki at google.com>
Date:   Sun Sep 25 16:35:56 2016 +0100

    Add to fuzz corpora for CVE-2016-6309
    
    Reviewed-by: Emilia Käsper <emilia at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>

commit 0d698f6696e114a6e47f8b75ff88ec81f9e30175
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Sep 23 16:58:11 2016 +0100

    Fix Use After Free for large message sizes
    
    The buffer to receive messages is initialised to 16k. If a message is
    received that is larger than that then the buffer is "realloc'd". This can
    cause the location of the underlying buffer to change. Anything that is
    referring to the old location will be referring to free'd data. In the
    recent commit c1ef7c97 (master) and 4b390b6c (1.1.0) the point in the code
    where the message buffer is grown was changed. However s->init_msg was not
    updated to point at the new location.
    
    CVE-2016-6309
    
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

commit f789b04f407c2003da62d2b91b587629f1a781d0
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Sep 23 16:41:50 2016 +0100

    Fix a WPACKET bug
    
    If we request more bytes to be allocated than double what we have already
    written, then we grow the buffer by the wrong amount.
    
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

commit 84d5549e692e63a16fa1b11603e4098fc31746e9
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Sep 23 15:37:13 2016 +0100

    Add a test for large messages
    
    Ensure that we send a large message during the test suite.
    
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 CHANGES                                            |  17 +++++
 NEWS                                               |   4 +
 .../06d05ea3d37abe7554e610be69b743585cb0c6fe}      | Bin 820 -> 921 bytes
 .../6b008546166c7e1d2ef100eb5ecbac7efe3b3b90       | Bin 0 -> 267 bytes
 .../f6b0502e2a8a63e84d7b474fad2b2dc127f12bac       | Bin 0 -> 267 bytes
 ssl/packet.c                                       |  10 ++-
 ssl/statem/statem.c                                |  20 ++++-
 test/sslapitest.c                                  |  84 +++++++++++++++++++++
 8 files changed, 129 insertions(+), 6 deletions(-)
 copy fuzz/corpora/{x509/3403363173e3b63d0b9f4e3fce6e8a734d946bfc => server/06d05ea3d37abe7554e610be69b743585cb0c6fe} (76%)
 create mode 100644 fuzz/corpora/server/6b008546166c7e1d2ef100eb5ecbac7efe3b3b90
 create mode 100644 fuzz/corpora/server/f6b0502e2a8a63e84d7b474fad2b2dc127f12bac

diff --git a/CHANGES b/CHANGES
index 97e70ac..eb18673 100644
--- a/CHANGES
+++ b/CHANGES
@@ -11,6 +11,23 @@
      https://www.akkadia.org/drepper/SHA-crypt.txt
      [Richard Levitte]
 
+ Changes between 1.1.0a and 1.1.0b [26 Sep 2016]
+
+  *) Fix Use After Free for large message sizes
+
+     The patch applied to address CVE-2016-6307 resulted in an issue where if a
+     message larger than approx 16k is received then the underlying buffer to
+     store the incoming message is reallocated and moved. Unfortunately a
+     dangling pointer to the old location is left which results in an attempt to
+     write to the previously freed location. This is likely to result in a
+     crash, however it could potentially lead to execution of arbitrary code.
+
+     This issue only affects OpenSSL 1.1.0a.
+
+     This issue was reported to OpenSSL by Robert Święcki.
+     (CVE-2016-6309)
+     [Matt Caswell]
+
  Changes between 1.1.0 and 1.1.0a [22 Sep 2016]
 
   *) OCSP Status Request extension unbounded memory growth
diff --git a/NEWS b/NEWS
index bdb7a4f..82d1cb1 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@
 
       o
 
+  Major changes between OpenSSL 1.1.0a and OpenSSL 1.1.0b [26 Sep 2016]
+
+      o Fix Use After Free for large message sizes (CVE-2016-6309)
+
   Major changes between OpenSSL 1.1.0 and OpenSSL 1.1.0a [22 Sep 2016]
 
       o OCSP Status Request extension unbounded memory growth (CVE-2016-6304)
diff --git a/fuzz/corpora/x509/3403363173e3b63d0b9f4e3fce6e8a734d946bfc b/fuzz/corpora/server/06d05ea3d37abe7554e610be69b743585cb0c6fe
similarity index 76%
copy from fuzz/corpora/x509/3403363173e3b63d0b9f4e3fce6e8a734d946bfc
copy to fuzz/corpora/server/06d05ea3d37abe7554e610be69b743585cb0c6fe
index 517c33c..42a535b 100644
Binary files a/fuzz/corpora/x509/3403363173e3b63d0b9f4e3fce6e8a734d946bfc and b/fuzz/corpora/server/06d05ea3d37abe7554e610be69b743585cb0c6fe differ
diff --git a/fuzz/corpora/server/6b008546166c7e1d2ef100eb5ecbac7efe3b3b90 b/fuzz/corpora/server/6b008546166c7e1d2ef100eb5ecbac7efe3b3b90
new file mode 100644
index 0000000..3b3f140
Binary files /dev/null and b/fuzz/corpora/server/6b008546166c7e1d2ef100eb5ecbac7efe3b3b90 differ
diff --git a/fuzz/corpora/server/f6b0502e2a8a63e84d7b474fad2b2dc127f12bac b/fuzz/corpora/server/f6b0502e2a8a63e84d7b474fad2b2dc127f12bac
new file mode 100644
index 0000000..abc8c3c
Binary files /dev/null and b/fuzz/corpora/server/f6b0502e2a8a63e84d7b474fad2b2dc127f12bac differ
diff --git a/ssl/packet.c b/ssl/packet.c
index 0e8e876..4077de5 100644
--- a/ssl/packet.c
+++ b/ssl/packet.c
@@ -24,12 +24,16 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t len, unsigned char **allocbytes)
 
     if (pkt->buf->length - pkt->written < len) {
         size_t newlen;
+        size_t reflen;
 
-        if (pkt->buf->length > SIZE_MAX / 2) {
+        reflen = (len > pkt->buf->length) ? len : pkt->buf->length;
+
+        if (reflen > SIZE_MAX / 2) {
             newlen = SIZE_MAX;
         } else {
-            newlen = (pkt->buf->length == 0) ? DEFAULT_BUF_SIZE
-                                             : pkt->buf->length * 2;
+            newlen = reflen * 2;
+            if (newlen < DEFAULT_BUF_SIZE)
+                newlen = DEFAULT_BUF_SIZE;
         }
         if (BUF_MEM_grow(pkt->buf, newlen) == 0)
             return 0;
diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c
index 27dd5d6..f004808 100644
--- a/ssl/statem/statem.c
+++ b/ssl/statem/statem.c
@@ -445,6 +445,21 @@ static void init_read_state_machine(SSL *s)
     st->read_state = READ_STATE_HEADER;
 }
 
+static int grow_init_buf(SSL *s, size_t size) {
+
+    size_t msg_offset = (char *)s->init_msg - s->init_buf->data;
+
+    if (!BUF_MEM_grow_clean(s->init_buf, (int)size))
+        return 0;
+
+    if (size < msg_offset)
+        return 0;
+
+    s->init_msg = s->init_buf->data + msg_offset;
+
+    return 1;
+}
+
 /*
  * This function implements the sub-state machine when the message flow is in
  * MSG_FLOW_READING. The valid sub-states and transitions are:
@@ -545,9 +560,8 @@ static SUB_STATE_RETURN read_state_machine(SSL *s)
             /* dtls_get_message already did this */
             if (!SSL_IS_DTLS(s)
                     && s->s3->tmp.message_size > 0
-                    && !BUF_MEM_grow_clean(s->init_buf,
-                                           (int)s->s3->tmp.message_size
-                                           + SSL3_HM_HEADER_LENGTH)) {
+                    && !grow_init_buf(s, s->s3->tmp.message_size
+                                         + SSL3_HM_HEADER_LENGTH)) {
                 ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
                 SSLerr(SSL_F_READ_STATE_MACHINE, ERR_R_BUF_LIB);
                 return SUB_STATE_ERROR;
diff --git a/test/sslapitest.c b/test/sslapitest.c
index a2b18df..acb2087 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -28,6 +28,88 @@ static int ocsp_client_called = 0;
 static int cdummyarg = 1;
 static X509 *ocspcert = NULL;
 
+#define NUM_EXTRA_CERTS 40
+
+static int execute_test_large_message(const SSL_METHOD *smeth,
+                                      const SSL_METHOD *cmeth)
+{
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0;
+    int i;
+    BIO *certbio = BIO_new_file(cert, "r");
+    X509 *chaincert = NULL;
+    int certlen;
+
+    if (certbio == NULL) {
+        printf("Can't load the certficate file\n");
+        goto end;
+    }
+    chaincert = PEM_read_bio_X509(certbio, NULL, NULL, NULL);
+
+    if (!create_ssl_ctx_pair(smeth, cmeth, &sctx,
+                             &cctx, cert, privkey)) {
+        printf("Unable to create SSL_CTX pair\n");
+        goto end;
+    }
+    BIO_free(certbio);
+    certbio = NULL;
+
+    /*
+     * We assume the supplied certificate is big enough so that if we add
+     * NUM_EXTRA_CERTS it will make the overall message large enough. The
+     * default buffer size is requested to be 16k, but due to the way BUF_MEM
+     * works, it ends up allocing a little over 21k (16 * 4/3). So, in this test
+     * we need to have a message larger than that.
+     */
+    certlen = i2d_X509(chaincert, NULL);
+    OPENSSL_assert((certlen * NUM_EXTRA_CERTS)
+                   > ((SSL3_RT_MAX_PLAIN_LENGTH * 4) / 3));
+    for (i = 0; i < NUM_EXTRA_CERTS; i++) {
+        if (!X509_up_ref(chaincert)) {
+            printf("Unable to up ref cert\n");
+            goto end;
+        }
+        if (!SSL_CTX_add_extra_chain_cert(sctx, chaincert)) {
+            printf("Unable to add extra chain cert %d\n", i);
+            X509_free(chaincert);
+            goto end;
+        }
+    }
+
+    if (!create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL, NULL)) {
+        printf("Unable to create SSL objects\n");
+        goto end;
+    }
+
+    if (!create_ssl_connection(serverssl, clientssl)) {
+        printf("Unable to create SSL connection\n");
+        goto end;
+    }
+
+    testresult = 1;
+
+ end:
+    X509_free(chaincert);
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+
+    return testresult;
+}
+
+static int test_large_message_tls(void)
+{
+    return execute_test_large_message(TLS_server_method(), TLS_client_method());
+}
+
+static int test_large_message_dtls(void)
+{
+    return execute_test_large_message(DTLS_server_method(),
+                                      DTLS_client_method());
+}
+
 static int ocsp_server_cb(SSL *s, void *arg)
 {
     int *argi = (int *)arg;
@@ -774,6 +856,8 @@ int main(int argc, char *argv[])
     CRYPTO_set_mem_debug(1);
     CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
 
+    ADD_TEST(test_large_message_tls);
+    ADD_TEST(test_large_message_dtls);
     ADD_TEST(test_tlsext_status_type);
     ADD_TEST(test_session_with_only_int_cache);
     ADD_TEST(test_session_with_only_ext_cache);


More information about the openssl-commits mailing list