From b51130dd5ce743f27762a0c1b9685b4ad5aa32d5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 27 Sep 2019 14:00:36 +0200 Subject: [PATCH 1/3] Parse HelloVerifyRequest: avoid buffer overread on the cookie In ssl_parse_hello_verify_request, we print cookie_len bytes without checking that there are that many bytes left in ssl->in_msg. This could potentially log data outside the received message (not a big deal) and could potentially read from memory outside of the receive buffer (which would be a remotely exploitable crash). --- library/ssl_cli.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 57e5d8ab9..9e0accc7e 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1605,8 +1605,6 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) } cookie_len = *p++; - MBEDTLS_SSL_DEBUG_BUF( 3, "cookie", p, cookie_len ); - if( ( ssl->in_msg + ssl->in_msglen ) - p < cookie_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, @@ -1615,6 +1613,7 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } + MBEDTLS_SSL_DEBUG_BUF( 3, "cookie", p, cookie_len ); mbedtls_free( ssl->handshake->verify_cookie ); From b64bf0638feba116beb8ee4e5e2145449de27127 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 27 Sep 2019 14:02:44 +0200 Subject: [PATCH 2/3] Parse HelloVerifyRequest: avoid buffer overread at the start In ssl_parse_hello_verify_request, we read 3 bytes (version and cookie length) without checking that there are that many bytes left in ssl->in_msg. This could potentially read from memory outside of the ssl->receive buffer (which would be a remotely exploitable crash). --- library/ssl_cli.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 9e0accc7e..af5ccf6cc 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1577,6 +1577,19 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse hello verify request" ) ); + /* Check that there is enough room for: + * - 2 bytes of version + * - 1 byte of cookie_len + */ + if( mbedtls_ssl_hs_hdr_len( ssl ) + 3 > ssl->in_msglen ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "incoming HelloVerifyRequest message is too short" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); + } + /* * struct { * ProtocolVersion server_version; From 1c668136afd261994af042d7d0585262c41fc64e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 27 Sep 2019 14:07:00 +0200 Subject: [PATCH 3/3] Parse HelloVerifyRequest buffer overread: add changelog entry --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index f16c97e8f..2dafb0e4b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 2.xx.x branch released xxxx-xx-xx + +Security + * Fix a potentially remotely exploitable buffer overread in a + DTLS client when parsing the Hello Verify Request message. + = mbed TLS 2.19.0 branch released 2019-09-06 Security