From 239987fd31255e2f8dc0fb03541d311deae6ee51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 9 Jan 2018 10:43:43 +0100 Subject: [PATCH] Fix heap-buffer overread in ALPN ext parsing --- ChangeLog | 3 +++ library/ssl_srv.c | 42 +++++++++++++++++++++++++----------------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3f5e56f9d..ef5abb8bc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -38,6 +38,9 @@ Security corrupt 6 bytes on the peer's heap, potentially leading to crash or remote code execution. This can be triggered remotely from either side in both TLS and DTLS. + * Fix a potential heap buffer overread in ALPN extension parsing + (server-side). Could result in application crash, but only if an ALPN + name larger than 16 bytes had been configured on the server. Features * Allow comments in test data files. diff --git a/library/ssl_srv.c b/library/ssl_srv.c index de3ea80e3..85c3c30ca 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -603,33 +603,41 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, } /* - * Use our order of preference + * Validate peer's list (lengths) */ start = buf + 2; end = buf + len; + for( theirs = start; theirs != end; theirs += cur_len ) + { + cur_len = *theirs++; + + /* Current identifier must fit in list */ + if( cur_len > (size_t)( end - theirs ) ) + { + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + + /* Empty strings MUST NOT be included */ + if( cur_len == 0 ) + { + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + } + + /* + * Use our order of preference + */ for( ours = ssl->conf->alpn_list; *ours != NULL; ours++ ) { ours_len = strlen( *ours ); for( theirs = start; theirs != end; theirs += cur_len ) { - /* If the list is well formed, we should get equality first */ - if( theirs > end ) - { - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); - } - cur_len = *theirs++; - /* Empty strings MUST NOT be included */ - if( cur_len == 0 ) - { - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); - } - if( cur_len == ours_len && memcmp( theirs, *ours, cur_len ) == 0 ) {