From 0c161d1956d72eb89c8660965535baeb2ec87dd3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 8 Oct 2018 13:40:50 +0100 Subject: [PATCH 1/3] Fix bounds check in ssl_parse_server_psk_hint() In the previous bounds check `(*p) > end - len`, the computation of `end - len` might underflow if `end` is within the first 64KB of the address space (note that the length `len` is controlled by the peer). In this case, the bounds check will be bypassed, leading to `*p` exceed the message bounds by up to 64KB when leaving `ssl_parse_server_psk_hint()`. In a pure PSK-based handshake, this doesn't seem to have any consequences, as `*p*` is not accessed afterwards. In a PSK-(EC)DHE handshake, however, `*p` is read from in `ssl_parse_server_ecdh_params()` and `ssl_parse_server_dh_params()` which might lead to an application crash of information leakage. --- library/ssl_cli.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 8385720115..b15bc515e8 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2097,7 +2097,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, * * opaque psk_identity_hint<0..2^16-1>; */ - if( (*p) > end - 2 ) + if( end - (*p) < 2 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " "(psk_identity_hint length)" ) ); @@ -2106,7 +2106,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, len = (*p)[0] << 8 | (*p)[1]; *p += 2; - if( (*p) > end - len ) + if( end - (*p) < len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " "(psk_identity_hint length)" ) ); From dc71ef8fcc448cf68c5ecf452d66378a1d189394 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 8 Oct 2018 13:51:38 +0100 Subject: [PATCH 2/3] Adapt ChangeLog --- ChangeLog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 513f24f3ab..24ab8af1b0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,13 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx +Security + * Fix a flawed bounds check in server PSK hint parsing. In case the + incoming message buffer was placed within the first 64KB of address + space and a PSK-(EC)DHE ciphersuite was used, this allowed an attacker + to trigger a memory access up to 64KB beyond the incoming message buffer, + potentially leading to application crash or information disclosure. + Bugfix * Fix a bug in the update function for SSL ticket keys which previously invalidated keys of a lifetime of less than a 1s. Fixes #1968. From 8df10232cf7bc6b82cbc3e19429fb551d36da97b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 10 Oct 2018 15:48:39 +0100 Subject: [PATCH 3/3] Add explicit unsigned-to-signed integer conversion The previous code triggered a compiler warning because of a comparison of a signed and an unsigned integer. The conversion is safe because `len` is representable by 16-bits, hence smaller than the maximum integer. --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index b15bc515e8..74883b3c3f 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2106,7 +2106,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, len = (*p)[0] << 8 | (*p)[1]; *p += 2; - if( end - (*p) < len ) + if( end - (*p) < (int) len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " "(psk_identity_hint length)" ) );