From bab079e85e7ec3ba476e9fc867f0c86d8d2a6b13 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 8e5c02b25..2be66ef85 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1884,7 +1884,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)" ) ); @@ -1893,7 +1893,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 be75866fb37a79302dc4ebe2f273ea4ffb2d6b84 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 8c82d08d1..3459d18a2 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 failure in hmac_drbg in the benchmark sample application, when MBEDTLS_THREADING_C is defined. Found by TrinityTonic, #1095 From e6a5ee7b7234b67c72ecb7850642ffe71bc0edb9 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 2be66ef85..0c3f1a878 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1893,7 +1893,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)" ) );