Fix length checking of various ClientKeyExchange's

ChangeLog

@@ -1,5 +1,10 @@
 PolarSSL ChangeLog (Sorted per branch, date)
 
+= PolarSSL 1.3 branch
+
+Bugfix
+   * The length of various ClientKeyExchange messages was not properly checked.
+
 = PolarSSL 1.3.5 released on 2014-03-26
 Features
    * HMAC-DRBG as a separate module

library/ecdh.c

@@ -218,10 +218,19 @@ int ecdh_make_public( ecdh_context *ctx, size_t *olen,
 int ecdh_read_public( ecdh_context *ctx,
                       const unsigned char *buf, size_t blen )
 {
+    int ret;
+    const unsigned char *p = buf;
+
     if( ctx == NULL )
         return( POLARSSL_ERR_ECP_BAD_INPUT_DATA );
 
-    return ecp_tls_read_point( &ctx->grp, &ctx->Qp, &buf, blen );
+    if( ( ret = ecp_tls_read_point( &ctx->grp, &ctx->Qp, &p, blen ) ) != 0 )
+        return( ret );
+
+    if( (size_t)( p - buf ) != blen )
+        return( POLARSSL_ERR_ECP_BAD_INPUT_DATA );
+
+    return( 0 );
 }
 
 /*

library/ssl_srv.c

@@ -2394,19 +2394,20 @@ static int ssl_parse_client_dh_public( ssl_context *ssl, unsigned char **p,
     n = ( (*p)[0] << 8 ) | (*p)[1];
     *p += 2;
 
-    if( n < 1 || n > ssl->handshake->dhm_ctx.len || *p + n > end )
+    if( *p + n > end )
     {
         SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) );
         return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
     }
 
-    if( ( ret = dhm_read_public( &ssl->handshake->dhm_ctx,
+    if( ( ret = dhm_read_public( &ssl->handshake->dhm_ctx, *p, n ) ) != 0 )
-                                  *p, n ) ) != 0 )
     {
         SSL_DEBUG_RET( 1, "dhm_read_public", ret );
         return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP );
     }
 
+    *p += n;
+
     SSL_DEBUG_MPI( 3, "DHM: GY", &ssl->handshake->dhm_ctx.GY );
 
     return( ret );
@@ -2583,7 +2584,7 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
     if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_DHE_RSA )
     {
         unsigned char *p = ssl->in_msg + 4;
-        unsigned char *end = ssl->in_msg + ssl->in_msglen;
+        unsigned char *end = ssl->in_msg + ssl->in_hslen;
 
         if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 )
         {
@@ -2591,6 +2592,12 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
             return( ret );
         }
 
+        if( p != end )
+        {
+            SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) );
+            return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
+        }
+
         ssl->handshake->pmslen = ssl->handshake->dhm_ctx.len;
 
         if( ( ret = dhm_calc_secret( &ssl->handshake->dhm_ctx,
@@ -2615,17 +2622,8 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
         ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDH_RSA ||
         ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDH_ECDSA )
     {
-        size_t n = ssl->in_msg[3];
-
-        if( n < 1 || n > mpi_size( &ssl->handshake->ecdh_ctx.grp.P ) * 2 + 2 ||
-            n + 4 != ssl->in_hslen )
-        {
-            SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) );
-            return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
-        }
-
         if( ( ret = ecdh_read_public( &ssl->handshake->ecdh_ctx,
-                                       ssl->in_msg + 4, n ) ) != 0 )
+                                       ssl->in_msg + 4, ssl->in_hslen - 4 ) ) != 0 )
         {
             SSL_DEBUG_RET( 1, "ecdh_read_public", ret );
             return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP );
@@ -2654,7 +2652,7 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
     if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_PSK )
     {
         unsigned char *p = ssl->in_msg + 4;
-        unsigned char *end = ssl->in_msg + ssl->in_msglen;
+        unsigned char *end = ssl->in_msg + ssl->in_hslen;
 
         if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 )
         {
@@ -2662,6 +2660,12 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
             return( ret );
         }
 
+        if( p != end )
+        {
+            SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) );
+            return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
+        }
+
         if( ( ret = ssl_psk_derive_premaster( ssl,
                         ciphersuite_info->key_exchange ) ) != 0 )
         {
@@ -2675,7 +2679,7 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
     if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_RSA_PSK )
     {
         unsigned char *p = ssl->in_msg + 4;
-        unsigned char *end = ssl->in_msg + ssl->in_msglen;
+        unsigned char *end = ssl->in_msg + ssl->in_hslen;
 
         if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 )
         {
@@ -2702,7 +2706,7 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
     if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_DHE_PSK )
     {
         unsigned char *p = ssl->in_msg + 4;
-        unsigned char *end = ssl->in_msg + ssl->in_msglen;
+        unsigned char *end = ssl->in_msg + ssl->in_hslen;
 
         if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 )
         {
@@ -2715,6 +2719,12 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
             return( ret );
         }
 
+        if( p != end )
+        {
+            SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) );
+            return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
+        }
+
         if( ( ret = ssl_psk_derive_premaster( ssl,
                         ciphersuite_info->key_exchange ) ) != 0 )
         {
@@ -2728,7 +2738,7 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
     if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDHE_PSK )
     {
         unsigned char *p = ssl->in_msg + 4;
-        unsigned char *end = ssl->in_msg + ssl->in_msglen;
+        unsigned char *end = ssl->in_msg + ssl->in_hslen;
 
         if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 )
         {
@@ -2759,10 +2769,10 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl )
     {
         if( ( ret = ssl_parse_encrypted_pms( ssl,
                                              ssl->in_msg + 4,
-                                             ssl->in_msg + ssl->in_msglen,
+                                             ssl->in_msg + ssl->in_hslen,
                                              0 ) ) != 0 )
         {
-            SSL_DEBUG_RET( 1, ( "ssl_parse_parse_ecrypted_pms_secret" ), ret );
+            SSL_DEBUG_RET( 1, ( "ssl_parse_parse_encrypted_pms_secret" ), ret );
             return( ret );
         }
     }