[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Wed Jun 10 09:38:58 UTC 2015


The branch master has been updated
       via  54e3ad003bdf83f189b2bf17fb998c028d39c8eb (commit)
       via  aec54108ef0d469964505ac1f77984f19099ec05 (commit)
       via  5d80fab086fe8849222613e20d7cf61839f94f5f (commit)
      from  e36c5fc4f547ce62280e5a704d1f94189742ec65 (commit)


- Log -----------------------------------------------------------------
commit 54e3ad003bdf83f189b2bf17fb998c028d39c8eb
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Apr 30 15:20:25 2015 +0100

    Tighten extension handling
    
    This adds additional checks to the processing of extensions in a ClientHello
    to ensure that either no extensions are present, or if they are then they
    take up the exact amount of space expected.
    
    With thanks to the Open Crypto Audit Project for reporting this issue.
    
    Reviewed-by: Stephen Henson <steve at openssl.org>

commit aec54108ef0d469964505ac1f77984f19099ec05
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Apr 30 14:51:10 2015 +0100

    Fix memory leaks in BIO_dup_chain()
    
    This fixes a memory leak that can occur whilst duplicating a BIO chain if
    the call to CRYPTO_dup_ex_data() fails. It also fixes a second memory leak
    where if a failure occurs after successfully creating the first BIO in the
    chain, then the beginning of the new chain was not freed.
    
    With thanks to the Open Crypto Audit Project for reporting this issue.
    
    Reviewed-by: Stephen Henson <steve at openssl.org>

commit 5d80fab086fe8849222613e20d7cf61839f94f5f
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Apr 30 14:04:30 2015 +0100

    Replace memset with OPENSSL_clear_free()
    
    BUF_MEM_free() attempts to cleanse memory using memset immediately prior
    to a free. This is at risk of being optimised away by the compiler, so
    replace with a call to OPENSSL_clear_free() instead.
    
    With thanks to the Open Crypto Audit Project for reporting this issue.
    
    Reviewed-by: Stephen Henson <steve at openssl.org>

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

Summary of changes:
 crypto/bio/bio_lib.c   |   7 ++-
 crypto/buffer/buffer.c |   3 +-
 ssl/t1_lib.c           | 158 ++++++++++++++++++++-----------------------------
 3 files changed, 70 insertions(+), 98 deletions(-)

diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
index 19cd069..cc859da 100644
--- a/crypto/bio/bio_lib.c
+++ b/crypto/bio/bio_lib.c
@@ -535,8 +535,10 @@ BIO *BIO_dup_chain(BIO *in)
 
         /* copy app data */
         if (!CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_BIO, &new_bio->ex_data,
-                                &bio->ex_data))
+                                &bio->ex_data)) {
+            BIO_free(new_bio);
             goto err;
+        }
 
         if (ret == NULL) {
             eoc = new_bio;
@@ -548,7 +550,8 @@ BIO *BIO_dup_chain(BIO *in)
     }
     return (ret);
  err:
-    BIO_free(ret);
+    BIO_free_all(ret);
+
     return (NULL);
 }
 
diff --git a/crypto/buffer/buffer.c b/crypto/buffer/buffer.c
index 37e5484..2beacce 100644
--- a/crypto/buffer/buffer.c
+++ b/crypto/buffer/buffer.c
@@ -88,8 +88,7 @@ void BUF_MEM_free(BUF_MEM *a)
         return;
 
     if (a->data != NULL) {
-        memset(a->data, 0, (unsigned int)a->max);
-        OPENSSL_free(a->data);
+        OPENSSL_clear_free(a->data, a->max);
     }
     OPENSSL_free(a);
 }
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 0420fe3..f0565a2 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1940,19 +1940,23 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
 
     s->srtp_profile = NULL;
 
-    if (data >= (d + n - 2))
-        goto ri_check;
+    if (data >= (d + n - 2)) {
+        if (data != d + n)
+            goto err;
+        else
+            goto ri_check;
+    }
     n2s(data, len);
 
     if (data > (d + n - len))
-        goto ri_check;
+        goto err;
 
     while (data <= (d + n - 4)) {
         n2s(data, type);
         n2s(data, size);
 
         if (data + size > (d + n))
-            goto ri_check;
+            goto err;
         if (s->tlsext_debug_cb)
             s->tlsext_debug_cb(s, 0, type, data, size, s->tlsext_debug_arg);
         if (type == TLSEXT_TYPE_renegotiate) {
@@ -1991,16 +1995,12 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
             int servname_type;
             int dsize;
 
-            if (size < 2) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (size < 2)
+                goto err;
             n2s(data, dsize);
             size -= 2;
-            if (dsize > size) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (dsize > size)
+                goto err;
 
             sdata = data;
             while (dsize > 3) {
@@ -2008,18 +2008,16 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
                 n2s(sdata, len);
                 dsize -= 3;
 
-                if (len > dsize) {
-                    *al = SSL_AD_DECODE_ERROR;
-                    return 0;
-                }
+                if (len > dsize)
+                    goto err;
+
                 if (s->servername_done == 0)
                     switch (servname_type) {
                     case TLSEXT_NAMETYPE_host_name:
                         if (!s->hit) {
-                            if (s->session->tlsext_hostname) {
-                                *al = SSL_AD_DECODE_ERROR;
-                                return 0;
-                            }
+                            if (s->session->tlsext_hostname)
+                                goto err;
+
                             if (len > TLSEXT_MAXLEN_host_name) {
                                 *al = TLS1_AD_UNRECOGNIZED_NAME;
                                 return 0;
@@ -2053,31 +2051,23 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
 
                 dsize -= len;
             }
-            if (dsize != 0) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (dsize != 0)
+                goto err;
 
         }
 #ifndef OPENSSL_NO_SRP
         else if (type == TLSEXT_TYPE_srp) {
-            if (size == 0 || ((len = data[0])) != (size - 1)) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
-            if (s->srp_ctx.login != NULL) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (size == 0 || ((len = data[0])) != (size - 1))
+                goto err;
+            if (s->srp_ctx.login != NULL)
+                goto err;
             if ((s->srp_ctx.login = OPENSSL_malloc(len + 1)) == NULL)
                 return -1;
             memcpy(s->srp_ctx.login, &data[1], len);
             s->srp_ctx.login[len] = '\0';
 
-            if (strlen(s->srp_ctx.login) != len) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (strlen(s->srp_ctx.login) != len)
+                goto err;
         }
 #endif
 
@@ -2087,10 +2077,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
             int ecpointformatlist_length = *(sdata++);
 
             if (ecpointformatlist_length != size - 1 ||
-                ecpointformatlist_length < 1) {
-                *al = TLS1_AD_DECODE_ERROR;
-                return 0;
-            }
+                ecpointformatlist_length < 1)
+                goto err;
             if (!s->hit) {
                 OPENSSL_free(s->session->tlsext_ecpointformatlist);
                 s->session->tlsext_ecpointformatlist = NULL;
@@ -2113,15 +2101,13 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
             if (ellipticcurvelist_length != size - 2 ||
                 ellipticcurvelist_length < 1 ||
                 /* Each NamedCurve is 2 bytes. */
-                ellipticcurvelist_length & 1) {
-                *al = TLS1_AD_DECODE_ERROR;
-                return 0;
-            }
+                ellipticcurvelist_length & 1)
+                    goto err;
+
             if (!s->hit) {
-                if (s->session->tlsext_ellipticcurvelist) {
-                    *al = TLS1_AD_DECODE_ERROR;
-                    return 0;
-                }
+                if (s->session->tlsext_ellipticcurvelist)
+                    goto err;
+
                 s->session->tlsext_ellipticcurvelist_length = 0;
                 if ((s->session->tlsext_ellipticcurvelist =
                      OPENSSL_malloc(ellipticcurvelist_length)) == NULL) {
@@ -2145,26 +2131,18 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
             }
         } else if (type == TLSEXT_TYPE_signature_algorithms) {
             int dsize;
-            if (s->s3->tmp.peer_sigalgs || size < 2) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (s->s3->tmp.peer_sigalgs || size < 2)
+                goto err;
             n2s(data, dsize);
             size -= 2;
-            if (dsize != size || dsize & 1 || !dsize) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
-            if (!tls1_save_sigalgs(s, data, dsize)) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (dsize != size || dsize & 1 || !dsize)
+                goto err;
+            if (!tls1_save_sigalgs(s, data, dsize))
+                goto err;
         } else if (type == TLSEXT_TYPE_status_request) {
 
-            if (size < 5) {
-                *al = SSL_AD_DECODE_ERROR;
-                return 0;
-            }
+            if (size < 5)
+                goto err;
 
             s->tlsext_status_type = *data++;
             size--;
@@ -2174,35 +2152,26 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
                 /* Read in responder_id_list */
                 n2s(data, dsize);
                 size -= 2;
-                if (dsize > size) {
-                    *al = SSL_AD_DECODE_ERROR;
-                    return 0;
-                }
+                if (dsize > size)
+                    goto err;
                 while (dsize > 0) {
                     OCSP_RESPID *id;
                     int idsize;
-                    if (dsize < 4) {
-                        *al = SSL_AD_DECODE_ERROR;
-                        return 0;
-                    }
+                    if (dsize < 4)
+                        goto err;
                     n2s(data, idsize);
                     dsize -= 2 + idsize;
                     size -= 2 + idsize;
-                    if (dsize < 0) {
-                        *al = SSL_AD_DECODE_ERROR;
-                        return 0;
-                    }
+                    if (dsize < 0)
+                        goto err;
                     sdata = data;
                     data += idsize;
                     id = d2i_OCSP_RESPID(NULL, &sdata, idsize);
-                    if (!id) {
-                        *al = SSL_AD_DECODE_ERROR;
-                        return 0;
-                    }
+                    if (!id)
+                        goto err;
                     if (data != sdata) {
                         OCSP_RESPID_free(id);
-                        *al = SSL_AD_DECODE_ERROR;
-                        return 0;
+                        goto err;
                     }
                     if (!s->tlsext_ocsp_ids
                         && !(s->tlsext_ocsp_ids =
@@ -2219,26 +2188,20 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
                 }
 
                 /* Read in request_extensions */
-                if (size < 2) {
-                    *al = SSL_AD_DECODE_ERROR;
-                    return 0;
-                }
+                if (size < 2)
+                    goto err;
                 n2s(data, dsize);
                 size -= 2;
-                if (dsize != size) {
-                    *al = SSL_AD_DECODE_ERROR;
-                    return 0;
-                }
+                if (dsize != size)
+                    goto err;
                 sdata = data;
                 if (dsize > 0) {
                     sk_X509_EXTENSION_pop_free(s->tlsext_ocsp_exts,
                                                X509_EXTENSION_free);
                     s->tlsext_ocsp_exts =
                         d2i_X509_EXTENSIONS(NULL, &sdata, dsize);
-                    if (!s->tlsext_ocsp_exts || (data + dsize != sdata)) {
-                        *al = SSL_AD_DECODE_ERROR;
-                        return 0;
-                    }
+                    if (!s->tlsext_ocsp_exts || (data + dsize != sdata))
+                        goto err;
                 }
             }
             /*
@@ -2329,6 +2292,10 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
         data += size;
     }
 
+    /* Spurious data on the end */
+    if (data != d + n)
+        goto err;
+
     *p = data;
 
  ri_check:
@@ -2344,6 +2311,9 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
     }
 
     return 1;
+err:
+    *al = SSL_AD_DECODE_ERROR;
+    return 0;
 }
 
 int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,


More information about the openssl-commits mailing list