[openssl-commits] [openssl] OpenSSL_1_0_1-stable update

Matt Caswell matt at openssl.org
Thu Apr 30 22:29:40 UTC 2015


The branch OpenSSL_1_0_1-stable has been updated
       via  017f695f2ca06ba45f6d9dd7be508934fb2a37e3 (commit)
       via  ee900ed1f7865d12682f5dd640d7554655cb4255 (commit)
       via  39b36cb438f7fba7dd3cce1d51d5c6c149f3e48d (commit)
       via  26800340dba2bf056d508007ee4d30e41d4e8f5f (commit)
       via  592ac25342a7863f38a3b316b183e90596f528b1 (commit)
       via  d889682208e4e75bca78f862b8e509a5b61b01f6 (commit)
       via  951ede2a06eba9a71c5d40b25f924e97f443c437 (commit)
       via  974d4d675cc6f3e1aa50b294ae12a5ba2acebd62 (commit)
       via  3be5df227259628cea91faffbea5054b872f793a (commit)
      from  80a06268ae329a1d7e01292029f9ae3af172b4b8 (commit)


- Log -----------------------------------------------------------------
commit 017f695f2ca06ba45f6d9dd7be508934fb2a37e3
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Apr 29 13:22:18 2015 +0100

    Fix buffer overrun in RSA signing
    
    The problem occurs in EVP_PKEY_sign() when using RSA with X931 padding.
    It is only triggered if the RSA key size is smaller than the digest length.
    So with SHA512 you can trigger the overflow with anything less than an RSA
    512 bit key. I managed to trigger a 62 byte overflow when using a 16 bit RSA
    key. This wasn't sufficient to cause a crash, although your mileage may
    vary.
    
    In practice RSA keys of this length are never used and X931 padding is very
    rare. Even if someone did use an excessively short RSA key, the chances of
    them combining that with a longer digest and X931 padding is very
    small. For these reasons I do not believe there is a security implication to
    this. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
    Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit 34166d41892643a36ad2d1f53cc0025e2edc2a39)

commit ee900ed1f7865d12682f5dd640d7554655cb4255
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Apr 29 09:58:10 2015 +0100

    Add sanity check to print_bin function
    
    Add a sanity check to the print_bin function to ensure that the |off|
    argument is positive. Thanks to Kevin Wojtysiak (Int3 Solutions) and
    Paramjot Oberoi (Int3 Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit 3deeeeb61b0c5b9b5f0993a67b7967d2f85186da)

commit 39b36cb438f7fba7dd3cce1d51d5c6c149f3e48d
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Apr 28 15:28:23 2015 +0100

    Add sanity check to ssl_get_prev_session
    
    Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
    Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
    reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit cb0f400b0cea2d2943f99b1e89c04ff6ed748cd5)

commit 26800340dba2bf056d508007ee4d30e41d4e8f5f
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Apr 28 15:19:50 2015 +0100

    Sanity check the return from final_finish_mac
    
    The return value is checked for 0. This is currently safe but we should
    really check for <= 0 since -1 is frequently used for error conditions.
    Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
    Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit c427570e5098e120cbcb66e799f85c317aac7b91)
    
    Conflicts:
    	ssl/ssl_locl.h
    
    Conflicts:
    	ssl/ssl_locl.h

commit 592ac25342a7863f38a3b316b183e90596f528b1
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Apr 27 15:41:42 2015 +0100

    Add sanity check in ssl3_cbc_digest_record
    
    For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
    this is true for all SSLv3 ciphersuites, this fact is far from obvious by
    looking at the code. If this were not the case then an integer overflow
    would occur, leading to a subsequent buffer overflow. Therefore I have
    added an explicit sanity check to ensure header_length is always valid.
    Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
    Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit 29b0a15a480626544dd0c803d5de671552544de6)

commit d889682208e4e75bca78f862b8e509a5b61b01f6
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Apr 27 15:41:03 2015 +0100

    Clarify logic in BIO_*printf functions
    
    The static function dynamically allocates an output buffer if the output
    grows larger than the static buffer that is normally used. The original
    logic implied that |currlen| could be greater than |maxlen| which is
    incorrect (and if so would cause a buffer overrun). Also the original
    logic would call OPENSSL_malloc to create a dynamic buffer equal to the
    size of the static buffer, and then immediately call OPENSSL_realloc to
    make it bigger, rather than just creating a buffer than was big enough in
    the first place. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
    Oberoi (Int3 Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit 9d9e37744cd5119f9921315864d1cd28717173cd)

commit 951ede2a06eba9a71c5d40b25f924e97f443c437
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Apr 27 11:13:56 2015 +0100

    Sanity check EVP_EncodeUpdate buffer len
    
    There was already a sanity check to ensure the passed buffer length is not
    zero. Extend this to ensure that it also not negative. Thanks to Kevin
    Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
    reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit b86d7dca69f5c80abd60896c8ed3039fc56210cc)

commit 974d4d675cc6f3e1aa50b294ae12a5ba2acebd62
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Apr 27 11:07:06 2015 +0100

    Sanity check EVP_CTRL_AEAD_TLS_AAD
    
    The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
    least 13 bytes long. Add sanity checks to ensure that the length is at
    least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
    represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
    Paramjot Oberoi (Int3 Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit c8269881093324b881b81472be037055571f73f3)
    
    Conflicts:
    	ssl/record/ssl3_record.c
    
    Conflicts:
    	apps/speed.c
    	crypto/evp/e_aes_cbc_hmac_sha256.c
    	crypto/evp/evp.h

commit 3be5df227259628cea91faffbea5054b872f793a
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Apr 27 11:04:56 2015 +0100

    Sanity check DES_enc_write buffer length
    
    Add a sanity check to DES_enc_write to ensure the buffer length provided
    is not negative. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
    Oberoi (Int3 Solutions) for reporting this issue.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (cherry picked from commit 873fb39f20b6763daba226b74e83fb194924c7bf)

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

Summary of changes:
 crypto/bio/b_print.c             | 45 +++++++++++++++++++---------------------
 crypto/des/enc_writ.c            |  3 +++
 crypto/ec/eck_prn.c              |  4 +++-
 crypto/evp/e_aes.c               |  2 +-
 crypto/evp/e_aes_cbc_hmac_sha1.c |  9 +++++---
 crypto/evp/e_rc4_hmac_md5.c      |  7 ++++++-
 crypto/evp/encode.c              |  2 +-
 crypto/evp/evp.h                 |  3 +++
 crypto/rsa/rsa_pmeth.c           |  8 ++++++-
 ssl/s3_both.c                    |  2 +-
 ssl/s3_cbc.c                     | 14 +++++++++++--
 ssl/ssl_sess.c                   |  2 +-
 ssl/t1_enc.c                     |  7 +++++--
 13 files changed, 70 insertions(+), 38 deletions(-)

diff --git a/crypto/bio/b_print.c b/crypto/bio/b_print.c
index 452e5cf..7c81e25 100644
--- a/crypto/bio/b_print.c
+++ b/crypto/bio/b_print.c
@@ -704,32 +704,29 @@ doapr_outch(char **sbuffer,
     /* If we haven't at least one buffer, someone has doe a big booboo */
     assert(*sbuffer != NULL || buffer != NULL);
 
-    if (buffer) {
-        while (*currlen >= *maxlen) {
-            if (*buffer == NULL) {
-                if (*maxlen == 0)
-                    *maxlen = 1024;
-                *buffer = OPENSSL_malloc(*maxlen);
-                if (!*buffer) {
-                    /* Panic! Can't really do anything sensible. Just return */
-                    return;
-                }
-                if (*currlen > 0) {
-                    assert(*sbuffer != NULL);
-                    memcpy(*buffer, *sbuffer, *currlen);
-                }
-                *sbuffer = NULL;
-            } else {
-                *maxlen += 1024;
-                *buffer = OPENSSL_realloc(*buffer, *maxlen);
-                if (!*buffer) {
-                    /* Panic! Can't really do anything sensible. Just return */
-                    return;
-                }
+    /* |currlen| must always be <= |*maxlen| */
+    assert(*currlen <= *maxlen);
+
+    if (buffer && *currlen == *maxlen) {
+        *maxlen += 1024;
+        if (*buffer == NULL) {
+            *buffer = OPENSSL_malloc(*maxlen);
+            if (!*buffer) {
+                /* Panic! Can't really do anything sensible. Just return */
+                return;
+            }
+            if (*currlen > 0) {
+                assert(*sbuffer != NULL);
+                memcpy(*buffer, *sbuffer, *currlen);
+            }
+            *sbuffer = NULL;
+        } else {
+            *buffer = OPENSSL_realloc(*buffer, *maxlen);
+            if (!*buffer) {
+                /* Panic! Can't really do anything sensible. Just return */
+                return;
             }
         }
-        /* What to do if *buffer is NULL? */
-        assert(*sbuffer != NULL || *buffer != NULL);
     }
 
     if (*currlen < *maxlen) {
diff --git a/crypto/des/enc_writ.c b/crypto/des/enc_writ.c
index 25041f2..bfaabde 100644
--- a/crypto/des/enc_writ.c
+++ b/crypto/des/enc_writ.c
@@ -96,6 +96,9 @@ int DES_enc_write(int fd, const void *_buf, int len,
     const unsigned char *cp;
     static int start = 1;
 
+    if (len < 0)
+        return -1;
+
     if (outbuf == NULL) {
         outbuf = OPENSSL_malloc(BSIZE + HDRSIZE);
         if (outbuf == NULL)
diff --git a/crypto/ec/eck_prn.c b/crypto/ec/eck_prn.c
index a911a0a..5ef12ec 100644
--- a/crypto/ec/eck_prn.c
+++ b/crypto/ec/eck_prn.c
@@ -338,12 +338,14 @@ static int print_bin(BIO *fp, const char *name, const unsigned char *buf,
 
     if (buf == NULL)
         return 1;
-    if (off) {
+    if (off > 0) {
         if (off > 128)
             off = 128;
         memset(str, ' ', off);
         if (BIO_write(fp, str, off) <= 0)
             return 0;
+    } else {
+        off = 0;
     }
 
     if (BIO_printf(fp, "%s", name) <= 0)
diff --git a/crypto/evp/e_aes.c b/crypto/evp/e_aes.c
index 245c18a..bde4804 100644
--- a/crypto/evp/e_aes.c
+++ b/crypto/evp/e_aes.c
@@ -753,7 +753,7 @@ static int aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
 
     case EVP_CTRL_AEAD_TLS1_AAD:
         /* Save the AAD for later use */
-        if (arg != 13)
+        if (arg != EVP_AEAD_TLS1_AAD_LEN)
             return 0;
         memcpy(c->buf, ptr, arg);
         gctx->tls_aad_len = arg;
diff --git a/crypto/evp/e_aes_cbc_hmac_sha1.c b/crypto/evp/e_aes_cbc_hmac_sha1.c
index 3f8a5ae..d1f5928 100644
--- a/crypto/evp/e_aes_cbc_hmac_sha1.c
+++ b/crypto/evp/e_aes_cbc_hmac_sha1.c
@@ -503,7 +503,12 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
     case EVP_CTRL_AEAD_TLS1_AAD:
         {
             unsigned char *p = ptr;
-            unsigned int len = p[arg - 2] << 8 | p[arg - 1];
+            unsigned int len;
+
+            if (arg != EVP_AEAD_TLS1_AAD_LEN)
+                return -1;
+ 
+            len = p[arg - 2] << 8 | p[arg - 1];
 
             if (ctx->encrypt) {
                 key->payload_length = len;
@@ -520,8 +525,6 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
                                AES_BLOCK_SIZE) & -AES_BLOCK_SIZE)
                              - len);
             } else {
-                if (arg > 13)
-                    arg = 13;
                 memcpy(key->aux.tls_aad, ptr, arg);
                 key->payload_length = arg;
 
diff --git a/crypto/evp/e_rc4_hmac_md5.c b/crypto/evp/e_rc4_hmac_md5.c
index 80735d3..e6b0cdf 100644
--- a/crypto/evp/e_rc4_hmac_md5.c
+++ b/crypto/evp/e_rc4_hmac_md5.c
@@ -258,7 +258,12 @@ static int rc4_hmac_md5_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
     case EVP_CTRL_AEAD_TLS1_AAD:
         {
             unsigned char *p = ptr;
-            unsigned int len = p[arg - 2] << 8 | p[arg - 1];
+            unsigned int len;
+
+            if (arg != EVP_AEAD_TLS1_AAD_LEN)
+                return -1;
+
+            len = p[arg - 2] << 8 | p[arg - 1];
 
             if (!ctx->encrypt) {
                 len -= MD5_DIGEST_LENGTH;
diff --git a/crypto/evp/encode.c b/crypto/evp/encode.c
index d1d8a07..5c5988f 100644
--- a/crypto/evp/encode.c
+++ b/crypto/evp/encode.c
@@ -137,7 +137,7 @@ void EVP_EncodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl,
     unsigned int total = 0;
 
     *outl = 0;
-    if (inl == 0)
+    if (inl <= 0)
         return;
     OPENSSL_assert(ctx->length <= (int)sizeof(ctx->enc_data));
     if ((ctx->num + inl) < ctx->length) {
diff --git a/crypto/evp/evp.h b/crypto/evp/evp.h
index b00997b..01bdeeb 100644
--- a/crypto/evp/evp.h
+++ b/crypto/evp/evp.h
@@ -409,6 +409,9 @@ struct evp_cipher_st {
 /* Set the GCM invocation field, decrypt only */
 # define         EVP_CTRL_GCM_SET_IV_INV         0x18
 
+/* RFC 5246 defines additional data to be 13 bytes in length */
+# define         EVP_AEAD_TLS1_AAD_LEN           13
+
 /* GCM TLS constants */
 /* Length of fixed part of IV derived from PRF */
 # define EVP_GCM_TLS_FIXED_IV_LEN                        4
diff --git a/crypto/rsa/rsa_pmeth.c b/crypto/rsa/rsa_pmeth.c
index d61d6e8..6a7c67c 100644
--- a/crypto/rsa/rsa_pmeth.c
+++ b/crypto/rsa/rsa_pmeth.c
@@ -228,8 +228,14 @@ static int pkey_rsa_sign(EVP_PKEY_CTX *ctx, unsigned char *sig,
                 return ret;
             ret = sltmp;
         } else if (rctx->pad_mode == RSA_X931_PADDING) {
-            if (!setup_tbuf(rctx, ctx))
+            if ((size_t)EVP_PKEY_size(ctx->pkey) < tbslen + 1) {
+                RSAerr(RSA_F_PKEY_RSA_SIGN, RSA_R_KEY_SIZE_TOO_SMALL);
+                return -1;
+            }
+            if (!setup_tbuf(rctx, ctx)) {
+                RSAerr(RSA_F_PKEY_RSA_SIGN, ERR_R_MALLOC_FAILURE);
                 return -1;
+            }
             memcpy(rctx->tbuf, tbs, tbslen);
             rctx->tbuf[tbslen] = RSA_X931_hash_id(EVP_MD_type(rctx->md));
             ret = RSA_private_encrypt(tbslen + 1, rctx->tbuf,
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 77374f4..107b460 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -169,7 +169,7 @@ int ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen)
         i = s->method->ssl3_enc->final_finish_mac(s,
                                                   sender, slen,
                                                   s->s3->tmp.finish_md);
-        if (i == 0)
+        if (i <= 0)
             return 0;
         s->s3->tmp.finish_md_len = i;
         memcpy(p, s->s3->tmp.finish_md, i);
diff --git a/ssl/s3_cbc.c b/ssl/s3_cbc.c
index 598d27e..00b534f 100644
--- a/ssl/s3_cbc.c
+++ b/ssl/s3_cbc.c
@@ -639,12 +639,22 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
 
     if (k > 0) {
         if (is_sslv3) {
+            unsigned overhang;
+
             /*
              * The SSLv3 header is larger than a single block. overhang is
              * the number of bytes beyond a single block that the header
-             * consumes: either 7 bytes (SHA1) or 11 bytes (MD5).
+             * consumes: either 7 bytes (SHA1) or 11 bytes (MD5). There are no
+             * ciphersuites in SSLv3 that are not SHA1 or MD5 based and
+             * therefore we can be confident that the header_length will be
+             * greater than |md_block_size|. However we add a sanity check just
+             * in case
              */
-            unsigned overhang = header_length - md_block_size;
+            if (header_length <= md_block_size) {
+                /* Should never happen */
+                return;
+            }
+            overhang = header_length - md_block_size;
             md_transform(md_state.c, header);
             memcpy(first_block, header + md_block_size, overhang);
             memcpy(first_block + overhang, data, md_block_size - overhang);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 4c7f5d8..eb7936b 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -478,7 +478,7 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
     int r;
 #endif
 
-    if (len > SSL_MAX_SSL_SESSION_ID_LENGTH)
+    if (len < 0 || len > SSL_MAX_SSL_SESSION_ID_LENGTH)
         goto err;
 
     if (session_id + len > limit) {
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 2736238..8f45294 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -785,7 +785,7 @@ int tls1_enc(SSL *s, int send)
         bs = EVP_CIPHER_block_size(ds->cipher);
 
         if (EVP_CIPHER_flags(ds->cipher) & EVP_CIPH_FLAG_AEAD_CIPHER) {
-            unsigned char buf[13], *seq;
+            unsigned char buf[EVP_AEAD_TLS1_AAD_LEN], *seq;
 
             seq = send ? s->s3->write_sequence : s->s3->read_sequence;
 
@@ -809,7 +809,10 @@ int tls1_enc(SSL *s, int send)
             buf[10] = (unsigned char)(s->version);
             buf[11] = rec->length >> 8;
             buf[12] = rec->length & 0xff;
-            pad = EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_AEAD_TLS1_AAD, 13, buf);
+            pad = EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_AEAD_TLS1_AAD,
+                                      EVP_AEAD_TLS1_AAD_LEN, buf);
+            if (pad <= 0)
+                return -1;
             if (send) {
                 l += pad;
                 rec->length += pad;


More information about the openssl-commits mailing list