From 01a96d6fd28996b0290d14dd853969d61be13184 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 afced7a99..eeedfe6cc 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1445,8 +1445,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, @@ -1455,6 +1453,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 d5c4a7cc11e7472e3b65bebbfce1fa0b953c178e 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 eeedfe6cc..c5c3af69d 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1417,6 +1417,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 afbcf97c2051f7f77db58463df4734cc1dd9a866 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 b296b814b..762f7615f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,8 @@ Security timings on the comparison in the key generation enabled the attacker to learn leading bits of the ephemeral key used during ECDSA signatures and to recover the private key. Reported by Jeremy Dubeuf. + * Fix a potentially remotely exploitable buffer overread in a + DTLS client when parsing the Hello Verify Request message. Bugfix * Remove redundant line for getting the bitlen of a bignum, since the variable