Be more strict when parsing TLS extensions.

Based on a diff from Kinichiro Inoguchi.

ok beck@

diff --git a/lib/libssl/src/ssl/t1_lib.c b/lib/libssl/src/ssl/t1_lib.c
index 7230dec..3022469 100644 (file)
--- a/lib/libssl/src/ssl/t1_lib.c
+++ b/lib/libssl/src/ssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.87 2016/05/30 13:42:54 beck Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.88 2016/08/27 15:58:06 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1206,6 +1206,7 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        unsigned short size;
        unsigned short len;
        unsigned char *data = *p;
+       unsigned char *end = d + n;
        int renegotiate_seen = 0;
        int sigalg_seen = 0;
 
@@ -1214,20 +1215,25 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        s->s3->next_proto_neg_seen = 0;
        free(s->s3->alpn_selected);
        s->s3->alpn_selected = NULL;
+       s->srtp_profile = NULL;
 
-       if (data >= (d + n - 2))
+       if (data == end)
                goto ri_check;
+
+       if (end - data < 2)
+               goto err;
        n2s(data, len);
 
-       if (data > (d + n - len))
-               goto ri_check;
+       if (end - data != len)
+               goto err;
 
-       while (data <= (d + n - 4)) {
+       while (end - data >= 4) {
                n2s(data, type);
                n2s(data, size);
 
-               if (data + size > (d + n))
-                       goto ri_check;
+               if (end - data < size)
+                       goto err;
+
                if (s->tlsext_debug_cb)
                        s->tlsext_debug_cb(s, 0, type, data, size,
                            s->tlsext_debug_arg);
@@ -1560,6 +1566,10 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
                data += size;
        }
 
+       /* Spurious data on the end */
+       if (data != end)
+               goto err;
+
        *p = data;
 
 ri_check:
@@ -1574,6 +1584,10 @@ ri_check:
        }
 
        return 1;
+
+err:
+       *al = SSL_AD_DECODE_ERROR;
+       return 0;
 }
 
 /*
@@ -1599,10 +1613,11 @@ int
 ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
     int n, int *al)
 {
-       unsigned short length;
        unsigned short type;
        unsigned short size;
+       unsigned short len;
        unsigned char *data = *p;
+       unsigned char *end = d + n;
        int tlsext_servername = 0;
        int renegotiate_seen = 0;
 
@@ -1610,21 +1625,22 @@ ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        free(s->s3->alpn_selected);
        s->s3->alpn_selected = NULL;
 
-       if (data >= (d + n - 2))
+       if (data == end)
                goto ri_check;
 
-       n2s(data, length);
-       if (data + length != d + n) {
-               *al = SSL_AD_DECODE_ERROR;
-               return 0;
-       }
+       if (end - data < 2)
+               goto err;
+       n2s(data, len);
+
+       if (end - data != len)
+               goto err;
 
-       while (data <= (d + n - 4)) {
+       while (end - data >= 4) {
                n2s(data, type);
                n2s(data, size);
 
-               if (data + size > (d + n))
-                       goto ri_check;
+               if (end - data < size)
+                       goto err;
 
                if (s->tlsext_debug_cb)
                        s->tlsext_debug_cb(s, 1, type, data, size,
@@ -1818,6 +1834,10 @@ ri_check:
        }
 
        return 1;
+
+err:
+       *al = SSL_AD_DECODE_ERROR;
+       return 0;
 }
 
 int

diff --git a/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 7230dec..3022469 100644 (file)
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.87 2016/05/30 13:42:54 beck Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.88 2016/08/27 15:58:06 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1206,6 +1206,7 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        unsigned short size;
        unsigned short len;
        unsigned char *data = *p;
+       unsigned char *end = d + n;
        int renegotiate_seen = 0;
        int sigalg_seen = 0;
 
@@ -1214,20 +1215,25 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        s->s3->next_proto_neg_seen = 0;
        free(s->s3->alpn_selected);
        s->s3->alpn_selected = NULL;
+       s->srtp_profile = NULL;
 
-       if (data >= (d + n - 2))
+       if (data == end)
                goto ri_check;
+
+       if (end - data < 2)
+               goto err;
        n2s(data, len);
 
-       if (data > (d + n - len))
-               goto ri_check;
+       if (end - data != len)
+               goto err;
 
-       while (data <= (d + n - 4)) {
+       while (end - data >= 4) {
                n2s(data, type);
                n2s(data, size);
 
-               if (data + size > (d + n))
-                       goto ri_check;
+               if (end - data < size)
+                       goto err;
+
                if (s->tlsext_debug_cb)
                        s->tlsext_debug_cb(s, 0, type, data, size,
                            s->tlsext_debug_arg);
@@ -1560,6 +1566,10 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
                data += size;
        }
 
+       /* Spurious data on the end */
+       if (data != end)
+               goto err;
+
        *p = data;
 
 ri_check:
@@ -1574,6 +1584,10 @@ ri_check:
        }
 
        return 1;
+
+err:
+       *al = SSL_AD_DECODE_ERROR;
+       return 0;
 }
 
 /*
@@ -1599,10 +1613,11 @@ int
 ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
     int n, int *al)
 {
-       unsigned short length;
        unsigned short type;
        unsigned short size;
+       unsigned short len;
        unsigned char *data = *p;
+       unsigned char *end = d + n;
        int tlsext_servername = 0;
        int renegotiate_seen = 0;
 
@@ -1610,21 +1625,22 @@ ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        free(s->s3->alpn_selected);
        s->s3->alpn_selected = NULL;
 
-       if (data >= (d + n - 2))
+       if (data == end)
                goto ri_check;
 
-       n2s(data, length);
-       if (data + length != d + n) {
-               *al = SSL_AD_DECODE_ERROR;
-               return 0;
-       }
+       if (end - data < 2)
+               goto err;
+       n2s(data, len);
+
+       if (end - data != len)
+               goto err;
 
-       while (data <= (d + n - 4)) {
+       while (end - data >= 4) {
                n2s(data, type);
                n2s(data, size);
 
-               if (data + size > (d + n))
-                       goto ri_check;
+               if (end - data < size)
+                       goto err;
 
                if (s->tlsext_debug_cb)
                        s->tlsext_debug_cb(s, 1, type, data, size,
@@ -1818,6 +1834,10 @@ ri_check:
        }
 
        return 1;
+
+err:
+       *al = SSL_AD_DECODE_ERROR;
+       return 0;
 }
 
 int