From 4bbaeb4ffafb57ca8e7901995ae274b05be4572b Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Thu, 22 Feb 2018 04:29:04 -0800 Subject: [PATCH 1/6] Add guard to out_left to avoid negative values return error when f_send return a value greater than out_left --- ChangeLog | 2 ++ library/ssl_tls.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/ChangeLog b/ChangeLog index 708ecad7e..d82600c07 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,6 +23,8 @@ Changes Contributed by Mathieu Briand. * Fix typo in a comment ctr_drbg.c. Contributed by Paul Sokolovsky. * Remove support for the library reference configuration for picocoin. + * Add guard to validate that out_left can not be negative. Raised by + samoconnor in #1245. = mbed TLS 2.7.0 branch released 2018-02-03 diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 617dedb1b..1de5eaab6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2469,6 +2469,12 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) if( ret <= 0 ) return( ret ); + if( (size_t)ret > ssl->out_left ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "f_send returned value greater than out left size" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + ssl->out_left -= ret; } From 5bd15cbfa09bc85b77c905ebff0bd5b57bab3888 Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Wed, 28 Feb 2018 04:30:59 -0800 Subject: [PATCH 2/6] Avoid wraparound for ssl->in_left Add check to avoid wraparound for ssl->in_left --- library/ssl_tls.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1de5eaab6..0d0660e6f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2422,6 +2422,14 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) if( ret < 0 ) return( ret ); + // At this point ret value is positive, verify that adding ret + // value to ssl->in_left doesn't cause a wraparound + if (ssl->in_left + (size_t)ret < ssl->in_left) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "wraparound happened over in_left value" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + ssl->in_left += ret; } } From b8788059196c901a263d4dc510c737fd009e47a4 Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Thu, 22 Mar 2018 02:40:43 -0700 Subject: [PATCH 3/6] Verify that f_send and f_recv send and receive the expected length Verify that f_send and f_recv send and receive the expected length --- ChangeLog | 5 +++-- library/ssl_tls.c | 12 +++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index d82600c07..71f69ee20 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,8 +23,9 @@ Changes Contributed by Mathieu Briand. * Fix typo in a comment ctr_drbg.c. Contributed by Paul Sokolovsky. * Remove support for the library reference configuration for picocoin. - * Add guard to validate that out_left can not be negative. Raised by - samoconnor in #1245. + * Verify that when (f_send, f_recv and f_recv_timeout) send or receive + more than the required length an error is returned. Raised by + Sam O'Connor in #1245. = mbed TLS 2.7.0 branch released 2018-02-03 diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0d0660e6f..2bd720410 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2422,11 +2422,11 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) if( ret < 0 ) return( ret ); - // At this point ret value is positive, verify that adding ret - // value to ssl->in_left doesn't cause a wraparound - if (ssl->in_left + (size_t)ret < ssl->in_left) + if ( (size_t)ret > len ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "wraparound happened over in_left value" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "f_recv returned %d bytes but only %zu were requested", + ret, len ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } @@ -2479,7 +2479,9 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) if( (size_t)ret > ssl->out_left ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "f_send returned value greater than out left size" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "f_send returned %d bytes but only %zu bytes were sent", + ret, ssl->out_left ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } From 52aecb9a7f57b63e56a8adde7baf75c9b60f5050 Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Wed, 28 Mar 2018 23:41:40 -0700 Subject: [PATCH 4/6] Check whether INT_MAX larger than SIZE_MAX scenario Check whether INT_MAX larger than SIZE_MAX scenario --- library/ssl_tls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2bd720410..a3515e1dc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2422,7 +2422,7 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) if( ret < 0 ) return( ret ); - if ( (size_t)ret > len ) + if ( (size_t)ret > len || ( INT_MAX > SIZE_MAX && ret > SIZE_MAX ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "f_recv returned %d bytes but only %zu were requested", @@ -2477,7 +2477,7 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) if( ret <= 0 ) return( ret ); - if( (size_t)ret > ssl->out_left ) + if( (size_t)ret > ssl->out_left || ( INT_MAX > SIZE_MAX && ret > SIZE_MAX ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "f_send returned %d bytes but only %zu bytes were sent", From 19d392b2581d1bded5ba61051b8b6343c0511b78 Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Mon, 2 Apr 2018 07:25:26 -0700 Subject: [PATCH 5/6] Fix compatibility problem in the printed message Replace %zu with %lu and add cast for the printed value. --- library/ssl_tls.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a3515e1dc..36899f3b8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2425,8 +2425,8 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) if ( (size_t)ret > len || ( INT_MAX > SIZE_MAX && ret > SIZE_MAX ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, - ( "f_recv returned %d bytes but only %zu were requested", - ret, len ) ); + ( "f_recv returned %d bytes but only %lu were requested", + ret, (unsigned long)len ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } @@ -2480,8 +2480,8 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) if( (size_t)ret > ssl->out_left || ( INT_MAX > SIZE_MAX && ret > SIZE_MAX ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, - ( "f_send returned %d bytes but only %zu bytes were sent", - ret, ssl->out_left ) ); + ( "f_send returned %d bytes but only %lu bytes were sent", + ret, (unsigned long)ssl->out_left ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } From d6953b58d74fb721edf71c825355d76d93b64129 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 4 Apr 2018 09:09:29 +0200 Subject: [PATCH 6/6] Improve changelog entry --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 71f69ee20..72f00e9fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,8 +23,8 @@ Changes Contributed by Mathieu Briand. * Fix typo in a comment ctr_drbg.c. Contributed by Paul Sokolovsky. * Remove support for the library reference configuration for picocoin. - * Verify that when (f_send, f_recv and f_recv_timeout) send or receive - more than the required length an error is returned. Raised by + * In the SSL module, when f_send, f_recv or f_recv_timeout report + transmitting more than the required length, return an error. Raised by Sam O'Connor in #1245. = mbed TLS 2.7.0 branch released 2018-02-03