From f65add4f60b94a3ee164209ee98aa0baf8c89fa0 Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Thu, 22 Feb 2018 04:29:04 -0800 Subject: [PATCH 1/6] Backport 2.1:Add guard to out_left to avoid negative values return error when f_send return a value greater than out_left --- ChangeLog | 6 ++++++ library/ssl_tls.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/ChangeLog b/ChangeLog index cf6331f340..a02f6df2c1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,8 +11,14 @@ Bugfix with flag MBEDTLS_X509_BADCERT_BAD_PK even when the key type was correct. In the context of SSL, this resulted in handshake failure. #1351 +Changes + * Add guard to validate that out_left can not be negative. Raised by + samoconnor in #1245. + + = mbed TLS 2.1.10 branch released 2018-02-03 + Security * Fix a heap corruption issue in the implementation of the truncated HMAC extension. When the truncated HMAC extension is enabled and CBC is used, diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7193809b13..7f51bee780 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2449,6 +2449,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 2b1eea72027168e3c3d8545d20686626f550272d Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Thu, 22 Feb 2018 05:13:34 -0800 Subject: [PATCH 2/6] Remove extra new lines Remove extra new lines --- ChangeLog | 2 -- 1 file changed, 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index a02f6df2c1..38e536da0a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,10 +15,8 @@ Changes * Add guard to validate that out_left can not be negative. Raised by samoconnor in #1245. - = mbed TLS 2.1.10 branch released 2018-02-03 - Security * Fix a heap corruption issue in the implementation of the truncated HMAC extension. When the truncated HMAC extension is enabled and CBC is used, From 89c12ecfb596a3ce050dba61b5c7c5c508a27d8f Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Mon, 19 Mar 2018 07:15:50 -0700 Subject: [PATCH 3/6] Avoid wraparound on in_left Avoid wraparound on 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 7f51bee780..3f1c406170 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2402,6 +2402,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 cee0890b19a4c3c0b9dec5ac0f8d10168b64cccb Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Thu, 22 Mar 2018 02:40:43 -0700 Subject: [PATCH 4/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 Conflicts: ChangeLog --- ChangeLog | 5 +++-- library/ssl_tls.c | 12 +++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 38e536da0a..f2c54f8bca 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,8 +12,9 @@ Bugfix In the context of SSL, this resulted in handshake failure. #1351 Changes - * 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.1.10 branch released 2018-02-03 diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3f1c406170..6ea70943f9 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2402,11 +2402,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 ); } @@ -2459,7 +2459,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 f72e51f2b8e0374e5bef5c420edf6a8c37fd6c6f Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Wed, 28 Mar 2018 23:44:39 -0700 Subject: [PATCH 5/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 6ea70943f9..12e9697952 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2402,7 +2402,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", @@ -2457,7 +2457,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 ad2908c9d680a99fcafcf771182b3e76f1ab332d Mon Sep 17 00:00:00 2001 From: mohammad1603 Date: Mon, 2 Apr 2018 07:30:32 -0700 Subject: [PATCH 6/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 12e9697952..4ac8186e24 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2405,8 +2405,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 ); } @@ -2460,8 +2460,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 ); }