From a47911cb70d1ff82f43bf7f3497dcda2340362f9 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 4 Jul 2018 17:41:58 +0200 Subject: [PATCH 1/4] Fix memory leak in ssl_setup --- library/ssl_tls.c | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 185f35ad1..f4a34b17c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5671,27 +5671,30 @@ void mbedtls_ssl_init( mbedtls_ssl_context *ssl ) int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, const mbedtls_ssl_config *conf ) { - int ret; + int err; + const size_t len = MBEDTLS_SSL_BUFFER_LEN; ssl->conf = conf; /* * Prepare base structures */ + ssl->out_buf = NULL; /* Set to NULL in case of an error condition */ + ssl->in_buf = mbedtls_calloc( 1, MBEDTLS_SSL_IN_BUFFER_LEN ); if( ssl->in_buf == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", MBEDTLS_SSL_IN_BUFFER_LEN) ); - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + err = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto error; } ssl->out_buf = mbedtls_calloc( 1, MBEDTLS_SSL_OUT_BUFFER_LEN ); if( ssl->out_buf == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", MBEDTLS_SSL_OUT_BUFFER_LEN) ); - mbedtls_free( ssl->in_buf ); - ssl->in_buf = NULL; - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + err = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto error; } #if defined(MBEDTLS_SSL_PROTO_DTLS) @@ -5725,10 +5728,33 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, ssl->in_msg = ssl->in_buf + 13; } - if( ( ret = ssl_handshake_init( ssl ) ) != 0 ) - return( ret ); + if( ( err = ssl_handshake_init( ssl ) ) != 0 ) + goto error; return( 0 ); + +error: + mbedtls_free( ssl->in_buf ); + mbedtls_free( ssl->out_buf ); + + ssl->conf = NULL; + + ssl->in_buf = NULL; + ssl->out_buf = NULL; + + ssl->in_hdr = NULL; + ssl->in_ctr = NULL; + ssl->in_len = NULL; + ssl->in_iv = NULL; + ssl->in_msg = NULL; + + ssl->out_hdr = NULL; + ssl->out_ctr = NULL; + ssl->out_len = NULL; + ssl->out_iv = NULL; + ssl->out_msg = NULL; + + return( err ); } /* From 21feae58cbc66c675e7ccf40ae1037ec7111cbd1 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Mon, 9 Jul 2018 14:42:35 +0200 Subject: [PATCH 2/4] Update change log --- ChangeLog | 5 +++++ library/ssl_tls.c | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 305eef60b..115f56ec8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,11 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx +Security + * Fix a potential memory leak in mbedtls_ssl_setup( ) function. An allocation + failure could leave an unreleased buffer. A handshake init failure would + lead to leaving two unreleased buffers. + Features * Add new crypto primitives from RFC 7539: stream cipher Chacha20, one-time authenticator Poly1305 and AEAD construct Chacha20-Poly1305. Contributed by diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f4a34b17c..661263abd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5672,7 +5672,6 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, const mbedtls_ssl_config *conf ) { int err; - const size_t len = MBEDTLS_SSL_BUFFER_LEN; ssl->conf = conf; From c9a5f02eabff9ee2440352b6c7fe084f713a6a27 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 24 Jul 2018 13:53:31 +0200 Subject: [PATCH 3/4] Move comment to a separate line --- library/ssl_tls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 661263abd..87af27402 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5678,7 +5678,9 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, /* * Prepare base structures */ - ssl->out_buf = NULL; /* Set to NULL in case of an error condition */ + + /* Set to NULL in case of an error condition */ + ssl->out_buf = NULL; ssl->in_buf = mbedtls_calloc( 1, MBEDTLS_SSL_IN_BUFFER_LEN ); if( ssl->in_buf == NULL ) From 9f7798ed3ffdc23359576ca84238cf4eef830599 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 31 Jul 2018 16:52:32 +0200 Subject: [PATCH 4/4] Revert change of a return variable name --- library/ssl_tls.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 87af27402..3327b2ca0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5671,7 +5671,7 @@ void mbedtls_ssl_init( mbedtls_ssl_context *ssl ) int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, const mbedtls_ssl_config *conf ) { - int err; + int ret; ssl->conf = conf; @@ -5686,7 +5686,7 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, if( ssl->in_buf == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", MBEDTLS_SSL_IN_BUFFER_LEN) ); - err = MBEDTLS_ERR_SSL_ALLOC_FAILED; + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; goto error; } @@ -5694,7 +5694,7 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, if( ssl->out_buf == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", MBEDTLS_SSL_OUT_BUFFER_LEN) ); - err = MBEDTLS_ERR_SSL_ALLOC_FAILED; + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; goto error; } @@ -5729,7 +5729,7 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, ssl->in_msg = ssl->in_buf + 13; } - if( ( err = ssl_handshake_init( ssl ) ) != 0 ) + if( ( ret = ssl_handshake_init( ssl ) ) != 0 ) goto error; return( 0 ); @@ -5755,7 +5755,7 @@ error: ssl->out_iv = NULL; ssl->out_msg = NULL; - return( err ); + return( ret ); } /*