From 99f3916e313763c3f49faf6c76bdc5677bc560dc Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 29 Jun 2018 09:04:46 +0100 Subject: [PATCH 1/3] Return from debugging functions if SSL context is unset The debugging functions - mbedtls_debug_print_ret, - mbedtls_debug_print_buf, - mbedtls_debug_print_mpi, and - mbedtls_debug_print_crt return immediately if the SSL configuration bound to the passed SSL context is NULL, has no debugging functions configured, or if the debug threshold is below the debugging level. However, they do not check whether the provided SSL context is not NULL before accessing the SSL configuration bound to it, therefore leading to a segmentation fault if it is. In contrast, the debugging function - mbedtls_debug_print_msg does check for ssl != NULL before accessing ssl->conf. This commit unifies the checks by always returning immediately if ssl == NULL. --- library/debug.c | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/library/debug.c b/library/debug.c index db3924ac5..30c8c7bb8 100644 --- a/library/debug.c +++ b/library/debug.c @@ -86,8 +86,13 @@ void mbedtls_debug_print_msg( const mbedtls_ssl_context *ssl, int level, char str[DEBUG_BUF_SIZE]; int ret; - if( NULL == ssl || NULL == ssl->conf || NULL == ssl->conf->f_dbg || level > debug_threshold ) + if( NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + level > debug_threshold ) + { return; + } va_start( argp, format ); #if defined(_WIN32) @@ -121,8 +126,13 @@ void mbedtls_debug_print_ret( const mbedtls_ssl_context *ssl, int level, { char str[DEBUG_BUF_SIZE]; - if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || level > debug_threshold ) + if( NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + level > debug_threshold ) + { return; + } /* * With non-blocking I/O and examples that just retry immediately, @@ -146,8 +156,13 @@ void mbedtls_debug_print_buf( const mbedtls_ssl_context *ssl, int level, char txt[17]; size_t i, idx = 0; - if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || level > debug_threshold ) + if( NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + level > debug_threshold ) + { return; + } mbedtls_snprintf( str + idx, sizeof( str ) - idx, "dumping '%s' (%u bytes)\n", text, (unsigned int) len ); @@ -199,8 +214,13 @@ void mbedtls_debug_print_ecp( const mbedtls_ssl_context *ssl, int level, { char str[DEBUG_BUF_SIZE]; - if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || level > debug_threshold ) + if( NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + level > debug_threshold ) + { return; + } mbedtls_snprintf( str, sizeof( str ), "%s(X)", text ); mbedtls_debug_print_mpi( ssl, level, file, line, str, &X->X ); @@ -219,8 +239,14 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level, int j, k, zeros = 1; size_t i, n, idx = 0; - if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || X == NULL || level > debug_threshold ) + if( NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + NULL == X || + level > debug_threshold ) + { return; + } for( n = X->n - 1; n > 0; n-- ) if( X->p[n] != 0 ) @@ -345,8 +371,14 @@ void mbedtls_debug_print_crt( const mbedtls_ssl_context *ssl, int level, char str[DEBUG_BUF_SIZE]; int i = 0; - if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || crt == NULL || level > debug_threshold ) + if( NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + NULL == crt || + level > debug_threshold ) + { return; + } while( crt != NULL ) { From 8b9d102160ba7e5939003588426f903fa8bbcfb8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 29 Jun 2018 09:10:39 +0100 Subject: [PATCH 2/3] Adapt ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 78e9ebf04..826252b0c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,8 @@ Bugfix interoperability issues with BouncyCastle. Raised by milenamil in #1157. * Fix potential use-after-free in mbedtls_ssl_get_max_frag_len() and mbedtls_ssl_get_record_expansion() after a session reset. Fixes #1941. + * Return from various debugging routines immediately if the + provided SSL context is unset. Changes * Improve compatibility with some alternative CCM implementations by using From b37ca7a4eb793bd58ab7ef33e6fadd129d0598a1 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 23 Aug 2018 16:40:43 +0100 Subject: [PATCH 3/3] Move ChangeLog entry from Bugfix to Changes section --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 826252b0c..d79b96ee3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,12 +12,12 @@ Bugfix interoperability issues with BouncyCastle. Raised by milenamil in #1157. * Fix potential use-after-free in mbedtls_ssl_get_max_frag_len() and mbedtls_ssl_get_record_expansion() after a session reset. Fixes #1941. - * Return from various debugging routines immediately if the - provided SSL context is unset. Changes * Improve compatibility with some alternative CCM implementations by using CCM test vectors from RAM. + * Return from various debugging routines immediately if the + provided SSL context is unset. = mbed TLS 2.7.5 branch released 2018-07-25