From 0f225c9a400dd5238c9517e6b2a3daefe4ce596a Mon Sep 17 00:00:00 2001 From: jsing Date: Sat, 27 Aug 2016 15:58:06 +0000 Subject: [PATCH] Be more strict when parsing TLS extensions. Based on a diff from Kinichiro Inoguchi. ok beck@ --- lib/libssl/src/ssl/t1_lib.c | 54 +++++++++++++++++++++++++------------ lib/libssl/t1_lib.c | 54 +++++++++++++++++++++++++------------ 2 files changed, 74 insertions(+), 34 deletions(-) diff --git a/lib/libssl/src/ssl/t1_lib.c b/lib/libssl/src/ssl/t1_lib.c index 7230dec6714..3022469ea91 100644 --- 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 7230dec6714..3022469ea91 100644 --- 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 -- 2.20.1