From ed9976634fef41e34b6f7ffe59d9703cc2c04dc2 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 28 Sep 2015 02:14:30 +0100 Subject: [PATCH] Added bounds checking for TLS extensions IOTSSL-478 - Added checks to prevent buffer overflows. --- library/ssl_cli.c | 111 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 108 insertions(+), 3 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index c82e2e70a..842f12c7f 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -60,6 +60,7 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; size_t hostname_len; *olen = 0; @@ -72,6 +73,13 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, hostname_len = strlen( ssl->hostname ); + if( (size_t)(end - p) < hostname_len + 9 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for hostname" ) ); + return; + } + + /* * struct { * NameType name_type; @@ -115,6 +123,7 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; *olen = 0; @@ -123,6 +132,12 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) ); + if( (size_t)(end - p) < 5 + ssl->verify_data_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + /* * Secure renegotiation */ @@ -149,6 +164,7 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; size_t sig_alg_len = 0; const int *md; #if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECDSA_C) @@ -162,9 +178,27 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) ); + for( md = ssl->conf->sig_hashes; *md != MBEDTLS_MD_NONE; md++ ) + { +#if defined(MBEDTLS_ECDSA_C) + sig_alg_len += 2; +#endif +#if defined(MBEDTLS_RSA_C) + sig_alg_len += 2; +#endif + } + + if( (size_t)(end - p) < sig_alg_len + 6 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + /* * Prepare signature_algorithms extension (TLS 1.2) */ + sig_alg_len = 0; + for( md = ssl->conf->sig_hashes; *md != MBEDTLS_MD_NONE; md++ ) { #if defined(MBEDTLS_ECDSA_C) @@ -214,6 +248,7 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; unsigned char *elliptic_curve_list = p + 6; size_t elliptic_curve_len = 0; const mbedtls_ecp_curve_info *info; @@ -227,6 +262,25 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_elliptic_curves extension" ) ); +#if defined(MBEDTLS_ECP_C) + for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) + { + info = mbedtls_ecp_curve_info_from_grp_id( *grp_id ); +#else + for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ ) + { +#endif + elliptic_curve_len += 2; + } + + if( (size_t)(end - p) < 6 + elliptic_curve_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + + elliptic_curve_len = 0; + #if defined(MBEDTLS_ECP_C) for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) { @@ -260,12 +314,18 @@ static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; - ((void) ssl); + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; *olen = 0; MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) ); + if( (size_t)(end - p) < 6 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS ) & 0xFF ); @@ -285,6 +345,7 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; if( ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE ) { *olen = 0; @@ -293,6 +354,12 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) ); + if( (size_t)(end - p) < 5 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH ) & 0xFF ); @@ -310,6 +377,7 @@ static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED ) { @@ -319,6 +387,12 @@ static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) ); + if( (size_t)(end - p) < 4 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC ) & 0xFF ); @@ -334,6 +408,7 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED || ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) @@ -345,6 +420,12 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac " "extension" ) ); + if( (size_t)(end - p) < 4 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC ) & 0xFF ); @@ -360,6 +441,7 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED || ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) @@ -371,6 +453,12 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret " "extension" ) ); + if( (size_t)(end - p) < 4 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET ) & 0xFF ); @@ -386,6 +474,7 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; size_t tlen = ssl->session_negotiate->ticket_len; if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED ) @@ -396,6 +485,12 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) ); + if( (size_t)(end - p) < 4 + tlen ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SESSION_TICKET >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SESSION_TICKET ) & 0xFF ); @@ -404,8 +499,7 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, *olen = 4; - if( ssl->session_negotiate->ticket == NULL || - ssl->session_negotiate->ticket_len == 0 ) + if( ssl->session_negotiate->ticket == NULL || tlen == 0 ) { return; } @@ -423,6 +517,8 @@ static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t *olen ) { unsigned char *p = buf; + const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; + size_t alpnlen = 0; const char **cur; if( ssl->conf->alpn_list == NULL ) @@ -433,6 +529,15 @@ static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding alpn extension" ) ); + for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) + alpnlen += *p = (unsigned char)( strlen( *cur ) & 0xFF ); + + if( (size_t)(end - p) < 6 + alpnlen ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return; + } + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ALPN >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ALPN ) & 0xFF );