From e1a9a4a82651ff43cf2cea7bc95867c590a99716 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 17 Oct 2017 18:15:41 +0300 Subject: [PATCH 1/2] Fix crash when calling `mbedtls_ssl_cache_free` twice Set `cache` to zero at the end of `mbedtls_ssl_cache_free` #1104 --- ChangeLog | 2 ++ library/ssl_cache.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/ChangeLog b/ChangeLog index c4e3998d0..44c2f78eb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,8 @@ Bugfix * Parse signature algorithm extension when renegotiating. Previously, renegotiated handshakes would only accept signatures using SHA-1 regardless of the peer's preferences, or fail if SHA-1 was disabled. + * Fix crash when calling mbedtls_ssl_cache_free() twice. Found by + MilenkoMitrovic, #1104 = mbed TLS 2.6.0 branch released 2017-08-10 diff --git a/library/ssl_cache.c b/library/ssl_cache.c index c771d7fe2..d34bc3d63 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -43,6 +43,11 @@ #include +/* Implementation that should never be optimized out by the compiler */ +static void mbedtls_zeroize( void *v, size_t n ) { + volatile unsigned char *p = v; while( n-- ) *p++ = 0; +} + void mbedtls_ssl_cache_init( mbedtls_ssl_cache_context *cache ) { memset( cache, 0, sizeof( mbedtls_ssl_cache_context ) ); @@ -321,6 +326,8 @@ void mbedtls_ssl_cache_free( mbedtls_ssl_cache_context *cache ) #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_free( &cache->mutex ); #endif + + mbedtls_zeroize( cache, sizeof(mbedtls_ssl_cache_context) ); } #endif /* MBEDTLS_SSL_CACHE_C */ From 22360825ae64374fb897d366c39f6704a56441b4 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 29 Oct 2017 17:53:52 +0200 Subject: [PATCH 2/2] Address PR review comments set `cache->chain` to NULL, instead of setting the whole structure to zero. --- library/ssl_cache.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/library/ssl_cache.c b/library/ssl_cache.c index d34bc3d63..47867f132 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -43,11 +43,6 @@ #include -/* Implementation that should never be optimized out by the compiler */ -static void mbedtls_zeroize( void *v, size_t n ) { - volatile unsigned char *p = v; while( n-- ) *p++ = 0; -} - void mbedtls_ssl_cache_init( mbedtls_ssl_cache_context *cache ) { memset( cache, 0, sizeof( mbedtls_ssl_cache_context ) ); @@ -326,8 +321,7 @@ void mbedtls_ssl_cache_free( mbedtls_ssl_cache_context *cache ) #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_free( &cache->mutex ); #endif - - mbedtls_zeroize( cache, sizeof(mbedtls_ssl_cache_context) ); + cache->chain = NULL; } #endif /* MBEDTLS_SSL_CACHE_C */