[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  5e0a80c1c9b2b06c2d203ad89778ce1b98e0b5ad (commit)
       via  5e9f0eebcfa25a55177d9a7025713262367bec14 (commit)
      from  e0e920b1a063f14f36418f8795c96f2c649400e1 (commit)


- Log -----------------------------------------------------------------
commit 5e0a80c1c9b2b06c2d203ad89778ce1b98e0b5ad
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Apr 10 16:49:33 2015 +0100

    Fix ssl_get_prev_session overrun
    
    If OpenSSL is configured with no-tlsext then ssl_get_prev_session can read
    past the end of the ClientHello message if the session_id length in the
    ClientHello is invalid. This should not cause any security issues since the
    underlying buffer is 16k in size. It should never be possible to overrun by
    that many bytes.
    
    This is probably made redundant by the previous commit - but you can never be
    too careful.
    
    With thanks to Qinghao Tang for reporting this issue.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 5e9f0eebcfa25a55177d9a7025713262367bec14
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Apr 10 17:25:27 2015 +0100

    Check for ClientHello message overruns
    
    The ClientHello processing is insufficiently rigorous in its checks to make
    sure that we don't read past the end of the message. This does not have
    security implications due to the size of the underlying buffer - but still
    needs to be fixed.
    
    With thanks to Qinghao Tang for reporting this issue.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

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

Summary of changes:
 ssl/s3_srvr.c  | 42 +++++++++++++++++++++++++++++++++++++++++-
 ssl/ssl_sess.c |  5 +++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 5b17e52..7376fe6 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -932,6 +932,16 @@ int ssl3_get_client_hello(SSL *s)
     d = p = (unsigned char *)s->init_msg;
 
     /*
+     * 2 bytes for client version, SSL3_RANDOM_SIZE bytes for random, 1 byte
+     * for session id length
+     */
+    if (n < 2 + SSL3_RANDOM_SIZE + 1) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+        goto f_err;
+    }
+
+    /*
      * use version from inside client hello, not from record header (may
      * differ: see RFC 2246, Appendix E, second paragraph)
      */
@@ -963,6 +973,12 @@ int ssl3_get_client_hello(SSL *s)
         unsigned int session_length, cookie_length;
 
         session_length = *(p + SSL3_RANDOM_SIZE);
+
+        if (p + SSL3_RANDOM_SIZE + session_length + 1 >= d + n) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
         cookie_length = *(p + SSL3_RANDOM_SIZE + session_length + 1);
 
         if (cookie_length == 0)
@@ -976,6 +992,12 @@ int ssl3_get_client_hello(SSL *s)
     /* get the session-id */
     j = *(p++);
 
+    if (p + j > d + n) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+        goto f_err;
+    }
+
     s->hit = 0;
     /*
      * Versions before 0.9.7 always allow clients to resume sessions in
@@ -1020,8 +1042,19 @@ int ssl3_get_client_hello(SSL *s)
 
     if (SSL_IS_DTLS(s)) {
         /* cookie stuff */
+        if (p + 1 > d + n) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
         cookie_len = *(p++);
 
+        if (p + cookie_len > d + n) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
+
         /*
          * The ClientHello may contain a cookie even if the
          * HelloVerify message has not been sent--make sure that it
@@ -1087,6 +1120,11 @@ int ssl3_get_client_hello(SSL *s)
         }
     }
 
+    if (p + 2 > d + n) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+        goto f_err;
+    }
     n2s(p, i);
     if ((i == 0) && (j != 0)) {
         /* we need a cipher if we are not resuming a session */
@@ -1094,7 +1132,9 @@ int ssl3_get_client_hello(SSL *s)
         SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_CIPHERS_SPECIFIED);
         goto f_err;
     }
-    if ((p + i) >= (d + n)) {
+
+    /* i bytes of cipher data + 1 byte for compression length later */
+    if ((p + i + 1) > (d + n)) {
         /* not enough data */
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index a213ea9..3d0f950 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -442,6 +442,11 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
     if (len > SSL_MAX_SSL_SESSION_ID_LENGTH)
         goto err;
 
+    if (session_id + len > limit) {
+        fatal = 1;
+        goto err;
+    }
+
     if (len == 0)
         try_session_cache = 0;