From 99b6777b72fd982d5d617f21286df58d8980344e 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 aaf42dd53..94e714e7e 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1439,8 +1439,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, @@ -1449,6 +1447,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 2414ce1a5e756acbebe01a7451629c2b0851d3a0 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 94e714e7e..ad40b1104 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1411,6 +1411,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 99258ff315783b40a6e903699fa00914952867b1 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 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 1f21f5e25..105875e49 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,8 @@ Security during certificate extensions parsing. In case of receiving malformed input (extensions length field equal to 0), an illegal read of one byte beyond the input buffer is made. Found and analyzed by Nathan Crandall. + * Fix a potentially remotely exploitable buffer overread in a + DTLS client when parsing the Hello Verify Request message. Bugfix * Fix a potential memory leak in mbedtls_ssl_setup() function. An allocation