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@

1 files changed, 9 insertions, 1 deletions

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;