From 747fd539380ed5d37e0927b4d2fb5326f2aca104 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Wed, 30 May 2018 09:13:21 +0200 Subject: [PATCH] Fixes different off by ones --- ChangeLog | 3 +++ library/ssl_cli.c | 6 +++--- library/ssl_srv.c | 43 ++++++++++++++++++++++++++++++++++++------- library/ssl_tls.c | 9 +++++++++ 4 files changed, 51 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8ebe9bb61..723539c39 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,9 @@ Changes * Support TLS testing in out-of-source builds using cmake. Fixes #1193. * Fix redundant declaration of mbedtls_ssl_list_ciphersuites. Raised by TrinityTonic. #1359. + * Adds of lengths checks in different functions (not a security issue as + original buffer is overgrown) thanks to Philippe Antoine from Catena + cyber. #1663. = mbed TLS 2.9.0 branch released 2018-04-30 diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 7455e99d2..f89972a4c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1247,14 +1247,14 @@ static int ssl_parse_supported_point_formats_ext( mbedtls_ssl_context *ssl, size_t list_size; const unsigned char *p; - list_size = buf[0]; - if( list_size + 1 != len ) + if( len == 0 || (size_t)( buf[0] + 1 ) != len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } + list_size = buf[0]; p = buf + 1; while( list_size > 0 ) @@ -2711,7 +2711,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) * therefore the buffer length at this point must be greater than that * regardless of the actual code path. */ - if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n ) + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 09b7a3fed..457f9bbc0 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -91,6 +91,13 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse ServerName extension" ) ); + if( len < 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } servername_list_size = ( ( buf[0] << 8 ) | ( buf[1] ) ); if( servername_list_size + 2 != len ) { @@ -101,7 +108,7 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, } p = buf + 2; - while( servername_list_size > 0 ) + while( servername_list_size > 2 ) { hostname_len = ( ( p[1] << 8 ) | p[2] ); if( hostname_len + 3 > servername_list_size ) @@ -205,6 +212,12 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, mbedtls_md_type_t md_cur; mbedtls_pk_type_t sig_cur; + if ( len < 2 ) { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } sig_alg_list_size = ( ( buf[0] << 8 ) | ( buf[1] ) ); if( sig_alg_list_size + 2 != len || sig_alg_list_size % 2 != 0 ) @@ -273,6 +286,12 @@ static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl, const unsigned char *p; const mbedtls_ecp_curve_info *curve_info, **curves; + if ( len < 2 ) { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } list_size = ( ( buf[0] << 8 ) | ( buf[1] ) ); if( list_size + 2 != len || list_size % 2 != 0 ) @@ -332,14 +351,14 @@ static int ssl_parse_supported_point_formats( mbedtls_ssl_context *ssl, size_t list_size; const unsigned char *p; - list_size = buf[0]; - if( list_size + 1 != len ) + if( len == 0 || (size_t)( buf[0] + 1 ) != len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } + list_size = buf[0]; p = buf + 1; while( list_size > 0 ) @@ -1656,10 +1675,16 @@ read_record_header: while( ext_len != 0 ) { - unsigned int ext_id = ( ( ext[0] << 8 ) - | ( ext[1] ) ); - unsigned int ext_size = ( ( ext[2] << 8 ) - | ( ext[3] ) ); + unsigned int ext_id; + unsigned int ext_size; + if ( ext_len < 4 ) { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ext_id = ( ( ext[0] << 8 ) | ( ext[1] ) ); + ext_size = ( ( ext[2] << 8 ) | ( ext[3] ) ); if( ext_size + 4 > ext_len ) { @@ -3299,6 +3324,10 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_0 ) { + if ( p + 2 > end ) { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); + } if( *p++ != ( ( len >> 8 ) & 0xFF ) || *p++ != ( ( len ) & 0xFF ) ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e8e0cd854..b8b8df205 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1151,6 +1151,9 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch * other_secret already set by the ClientKeyExchange message, * and is 48 bytes long */ + if( end - p < 2 ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + *p++ = 0; *p++ = 48; p += 48; @@ -4528,6 +4531,12 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ) while( i < ssl->in_hslen ) { + if ( i + 3 > ssl->in_hslen ) { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ); + } if( ssl->in_msg[i] != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate message" ) );