From 40e26b2600ff2afcd417549194aa3393f6675763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 6 Jul 2021 12:05:23 +0200 Subject: [PATCH 1/2] Fix memory leak on failure path in test code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_test_common_source.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index d9e36078d..a09e6ed19 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -185,7 +185,8 @@ int ssl_check_record( mbedtls_ssl_context const *ssl, if( ret != ret_repeated ) { mbedtls_printf( "mbedtls_ssl_check_record() returned inconsistent results.\n" ); - return( -1 ); + ret = -1; + goto cleanup; } switch( ret ) @@ -196,29 +197,34 @@ int ssl_check_record( mbedtls_ssl_context const *ssl, case MBEDTLS_ERR_SSL_INVALID_RECORD: if( opt.debug_level > 1 ) mbedtls_printf( "mbedtls_ssl_check_record() detected invalid record.\n" ); + ret = 0; break; case MBEDTLS_ERR_SSL_INVALID_MAC: if( opt.debug_level > 1 ) mbedtls_printf( "mbedtls_ssl_check_record() detected unauthentic record.\n" ); + ret = 0; break; case MBEDTLS_ERR_SSL_UNEXPECTED_RECORD: if( opt.debug_level > 1 ) mbedtls_printf( "mbedtls_ssl_check_record() detected unexpected record.\n" ); + ret = 0; break; default: mbedtls_printf( "mbedtls_ssl_check_record() failed fatally with -%#04x.\n", (unsigned int) -ret ); - return( -1 ); + ret = -1; + goto cleanup; } /* Regardless of the outcome, forward the record to the stack. */ } +cleanup: mbedtls_free( tmp_buf ); - return( 0 ); + return( ret ); } #endif /* MBEDTLS_SSL_RECORD_CHECKING */ From 87e8b5ccaa6470a95a7de5057cec493e72ebf936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 7 Jul 2021 10:48:26 +0200 Subject: [PATCH 2/2] Use distinct variables for distinct purposes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_test_common_source.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index a09e6ed19..f5d868046 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -163,7 +163,7 @@ int dtls_srtp_key_derivation( void *p_expkey, int ssl_check_record( mbedtls_ssl_context const *ssl, unsigned char const *buf, size_t len ) { - int ret; + int my_ret = 0, ret_cr1, ret_cr2; unsigned char *tmp_buf; /* Record checking may modify the input buffer, @@ -173,23 +173,21 @@ int ssl_check_record( mbedtls_ssl_context const *ssl, return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); memcpy( tmp_buf, buf, len ); - ret = mbedtls_ssl_check_record( ssl, tmp_buf, len ); - if( ret != MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ) + ret_cr1 = mbedtls_ssl_check_record( ssl, tmp_buf, len ); + if( ret_cr1 != MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ) { - int ret_repeated; - /* Test-only: Make sure that mbedtls_ssl_check_record() * doesn't alter state. */ memcpy( tmp_buf, buf, len ); /* Restore buffer */ - ret_repeated = mbedtls_ssl_check_record( ssl, tmp_buf, len ); - if( ret != ret_repeated ) + ret_cr2 = mbedtls_ssl_check_record( ssl, tmp_buf, len ); + if( ret_cr2 != ret_cr1 ) { mbedtls_printf( "mbedtls_ssl_check_record() returned inconsistent results.\n" ); - ret = -1; + my_ret = -1; goto cleanup; } - switch( ret ) + switch( ret_cr1 ) { case 0: break; @@ -197,24 +195,21 @@ int ssl_check_record( mbedtls_ssl_context const *ssl, case MBEDTLS_ERR_SSL_INVALID_RECORD: if( opt.debug_level > 1 ) mbedtls_printf( "mbedtls_ssl_check_record() detected invalid record.\n" ); - ret = 0; break; case MBEDTLS_ERR_SSL_INVALID_MAC: if( opt.debug_level > 1 ) mbedtls_printf( "mbedtls_ssl_check_record() detected unauthentic record.\n" ); - ret = 0; break; case MBEDTLS_ERR_SSL_UNEXPECTED_RECORD: if( opt.debug_level > 1 ) mbedtls_printf( "mbedtls_ssl_check_record() detected unexpected record.\n" ); - ret = 0; break; default: - mbedtls_printf( "mbedtls_ssl_check_record() failed fatally with -%#04x.\n", (unsigned int) -ret ); - ret = -1; + mbedtls_printf( "mbedtls_ssl_check_record() failed fatally with -%#04x.\n", (unsigned int) -ret_cr1 ); + my_ret = -1; goto cleanup; } @@ -224,7 +219,7 @@ int ssl_check_record( mbedtls_ssl_context const *ssl, cleanup: mbedtls_free( tmp_buf ); - return( ret ); + return( my_ret ); } #endif /* MBEDTLS_SSL_RECORD_CHECKING */