From c95157e4b6c5e281cb496ef41f9969df25abef91 Mon Sep 17 00:00:00 2001 From: miod <> Date: Fri, 11 Jul 2014 22:57:25 +0000 Subject: As reported by David Ramos, most consumer of ssl_get_message() perform late bounds check, after reading the 2-, 3- or 4-byte size of the next chunk to process. But the size fields themselves are not checked for being entirely contained in the buffer. Since reading past your bounds is bad practice, and may not possible if you are using a secure memory allocator, we need to add the necessary bounds check, at the expense of some readability. As a bonus, a wrong size GOST session key will now trigger an error instead of a printf to stderr and it being handled as if it had the correct size. Creating this diff made my eyes bleed (in the real sense); reviewing it made guenther@'s and beck@'s eyes bleed too (in the literal sense). ok guenther@ beck@ --- src/lib/libssl/d1_clnt.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/lib/libssl/d1_clnt.c') diff --git a/src/lib/libssl/d1_clnt.c b/src/lib/libssl/d1_clnt.c index 3f47a3854b..b85908c733 100644 --- a/src/lib/libssl/d1_clnt.c +++ b/src/lib/libssl/d1_clnt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_clnt.c,v 1.28 2014/07/11 09:24:44 beck Exp $ */ +/* $OpenBSD: d1_clnt.c,v 1.29 2014/07/11 22:57:25 miod Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -879,6 +879,8 @@ dtls1_get_hello_verify(SSL *s) return (1); } + if (2 > n) + goto truncated; data = (unsigned char *)s->init_msg; if ((data[0] != (s->version >> 8)) || (data[1] != (s->version&0xff))) { @@ -889,7 +891,11 @@ dtls1_get_hello_verify(SSL *s) } data += 2; + if (2 + 1 > n) + goto truncated; cookie_len = *(data++); + if (2 + 1 + cookie_len > n) + goto truncated; if (cookie_len > sizeof(s->d1->cookie)) { al = SSL_AD_ILLEGAL_PARAMETER; goto f_err; @@ -901,6 +907,8 @@ dtls1_get_hello_verify(SSL *s) s->d1->send_cookie = 1; return 1; +truncated: + al = SSL_AD_DECODE_ERROR; f_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); return -1; -- cgit v1.2.3-55-g6feb