commit 0fc94e9f8338ed13d1a94ec2b42e2d20051edecb
author Simon Butcher <simon.butcher@arm.com> Mon Sep 28 20:52:04 2015 +0100
committer Simon Butcher <simon.butcher@arm.com> Mon Sep 28 20:52:04 2015 +0100
tree f5455029e5d2bc00c056c40f1b2085a82846d632
parent 9f81231fb84736c11fdae47b2f707c6e3127ddbb

Revised bounds checking on TLS extensions

Revisions following review feedback

library/ssl_cli.c

diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 842f12c..825a74f 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -73,13 +73,12 @@
 
     hostname_len = strlen( ssl->hostname );
 
-    if( (size_t)(end - p) < hostname_len + 9 )
+    if( end < p || (size_t)( end - p ) < hostname_len + 9 )
     {
-        MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for hostname" ) );
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
     }
 
-
     /*
      * struct {
      *     NameType name_type;
@@ -132,7 +131,7 @@
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) );
 
-    if( (size_t)(end - p) < 5 + ssl->verify_data_len )
+    if( end < p || (size_t)( end - p ) < 5 + ssl->verify_data_len )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -188,7 +187,7 @@
 #endif
     }
 
-    if( (size_t)(end - p) < sig_alg_len + 6 )
+    if( end < p || (size_t)( end - p ) < sig_alg_len + 6 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -273,7 +272,7 @@
         elliptic_curve_len += 2;
     }
 
-    if( (size_t)(end - p) < 6 + elliptic_curve_len )
+    if( end < p || (size_t)( end - p ) < 6 + elliptic_curve_len )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -320,7 +319,7 @@
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) );
 
-    if( (size_t)(end - p) < 6 )
+    if( end < p || (size_t)( end - p ) < 6 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -347,14 +346,15 @@
     unsigned char *p = buf;
     const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
 
+    *olen = 0;
+
     if( ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE ) {
-        *olen = 0;
         return;
     }
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) );
 
-    if( (size_t)(end - p) < 5 )
+    if( end < p || (size_t)( end - p ) < 5 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -379,15 +379,16 @@
     unsigned char *p = buf;
     const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
 
+    *olen = 0;
+
     if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED )
     {
-        *olen = 0;
         return;
     }
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) );
 
-    if( (size_t)(end - p) < 4 )
+    if( end < p || (size_t)( end - p ) < 4 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -410,17 +411,18 @@
     unsigned char *p = buf;
     const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
 
+    *olen = 0;
+
     if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED ||
         ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 )
     {
-        *olen = 0;
         return;
     }
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac "
                         "extension" ) );
 
-    if( (size_t)(end - p) < 4 )
+    if( end < p || (size_t)( end - p ) < 4 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -443,17 +445,18 @@
     unsigned char *p = buf;
     const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
 
+    *olen = 0;
+
     if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED ||
         ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 )
     {
-        *olen = 0;
         return;
     }
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret "
                         "extension" ) );
 
-    if( (size_t)(end - p) < 4 )
+    if( end < p || (size_t)( end - p ) < 4 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -477,15 +480,16 @@
     const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
     size_t tlen = ssl->session_negotiate->ticket_len;
 
+    *olen = 0;
+
     if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED )
     {
-        *olen = 0;
         return;
     }
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) );
 
-    if( (size_t)(end - p) < 4 + tlen )
+    if( end < p || (size_t)( end - p ) < 4 + tlen )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;
@@ -521,9 +525,10 @@
     size_t alpnlen = 0;
     const char **cur;
 
+    *olen = 0;
+
     if( ssl->conf->alpn_list == NULL )
     {
-        *olen = 0;
         return;
     }
 
@@ -532,7 +537,7 @@
     for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ )
         alpnlen +=   *p = (unsigned char)( strlen( *cur ) & 0xFF );
 
-    if( (size_t)(end - p) < 6 + alpnlen )
+    if( end < p || (size_t)( end - p ) < 6 + alpnlen )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
         return;