diff --git a/ChangeLog b/ChangeLog index ae8d86f20..3e5dd6808 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,12 @@ Security a non DER-compliant certificate correctly signed by a trusted CA, or a trusted CA with a non DER-compliant certificate. Found by luocm on GitHub. Fixes #825. + * Fix buffer length assertion in the ssl_parse_certificate_request() + function which leads to an arbitrary overread of the message buffer. The + overreads could occur upon receiving a message malformed at the point + where an optional signature algorithms list is expected in the cases of + the signature algorithms section being too short. In the debug builds + the overread data is printed to the standard output. Features * Add option MBEDTLS_AES_FEWER_TABLES to dynamically compute 3/4 of the AES tables @@ -55,6 +61,9 @@ Bugfix in the internal buffers; these cases lead to deadlocks in case event-driven I/O was used. Found and reported by Hubert Mis in #772. + * Fix buffer length assertions in the ssl_parse_certificate_request() + function which leads to a potential one byte overread of the message + buffer. Changes * Remove some redundant code in bignum.c. Contributed by Alexey Skalozub. diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 738014e9e..7cde5b113 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2673,10 +2673,27 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) buf = ssl->in_msg; /* certificate_types */ + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE_REQUEST ); + } cert_type_len = buf[mbedtls_ssl_hs_hdr_len( ssl )]; n = cert_type_len; - if( ssl->in_hslen < mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n ) + /* + * In the subsequent code there are two paths that read from buf: + * * the length of the signature algorithms field (if minor version of + * SSL is 3), + * * distinguished name length otherwise. + * Both reach at most the index: + * ...hdr_len + 2 + n, + * therefore the buffer length at this point must be greater than that + * regardless of the actual code path. + */ + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, @@ -2691,9 +2708,32 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) size_t sig_alg_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); #if defined(MBEDTLS_DEBUG_C) - unsigned char* sig_alg = buf + mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n; + unsigned char* sig_alg; size_t i; +#endif + /* + * The furthest access in buf is in the loop few lines below: + * sig_alg[i + 1], + * where: + * sig_alg = buf + ...hdr_len + 3 + n, + * max(i) = sig_alg_len - 1. + * Therefore the furthest access is: + * buf[...hdr_len + 3 + n + sig_alg_len - 1 + 1], + * which reduces to: + * buf[...hdr_len + 3 + n + sig_alg_len], + * which is one less than we need the buf to be. + */ + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n + sig_alg_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE_REQUEST ); + } + +#if defined(MBEDTLS_DEBUG_C) + sig_alg = buf + mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n; for( i = 0; i < sig_alg_len; i += 2 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Signature Algorithm found: %d" @@ -2702,14 +2742,6 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) #endif n += 2 + sig_alg_len; - - if( ssl->in_hslen < mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE_REQUEST ); - } } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */