From 687c500da335d0082f7e310c3f8b3cabc2d48a6c 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 485aaaf2b9e44a807232cbf504ced924b7447acd 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 0598cfa1a..fd25ac728 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,6 +13,8 @@ Bugfix * Replace printf with mbedtls_printf in aria. Found by TrinityTonic in #1908. * 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 * Copy headers preserving timestamps when doing a "make install". From aa035d89a66736d119e9f750cf4324dfe69a26b5 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 fd25ac728..efaefa7df 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,8 +13,6 @@ Bugfix * Replace printf with mbedtls_printf in aria. Found by TrinityTonic in #1908. * 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 * Copy headers preserving timestamps when doing a "make install". @@ -23,6 +21,8 @@ Changes Drozd. Fixes #1215 raised by randombit. * 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.12.0 branch released 2018-07-25