Revised bounds checking on TLS extensions

Revisions following review feedback
This commit is contained in:
Simon Butcher 2015-09-28 20:52:04 +01:00
parent 9f81231fb8
commit 0fc94e9f83

View File

@ -73,13 +73,12 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl,
hostname_len = strlen( ssl->hostname ); hostname_len = strlen( ssl->hostname );
if( (size_t)(end - p) < hostname_len + 9 ) if( end < p || (size_t)( end - p ) < hostname_len + 9 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for hostname" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
} }
/* /*
* struct { * struct {
* NameType name_type; * NameType name_type;
@ -132,7 +131,7 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) );
if( (size_t)(end - p) < 5 + ssl->verify_data_len ) if( end < p || (size_t)( end - p ) < 5 + ssl->verify_data_len )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -188,7 +187,7 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl,
#endif #endif
} }
if( (size_t)(end - p) < sig_alg_len + 6 ) if( end < p || (size_t)( end - p ) < sig_alg_len + 6 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -273,7 +272,7 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl,
elliptic_curve_len += 2; elliptic_curve_len += 2;
} }
if( (size_t)(end - p) < 6 + elliptic_curve_len ) if( end < p || (size_t)( end - p ) < 6 + elliptic_curve_len )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -320,7 +319,7 @@ static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) );
if( (size_t)(end - p) < 6 ) if( end < p || (size_t)( end - p ) < 6 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -347,14 +346,15 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl,
unsigned char *p = buf; unsigned char *p = buf;
const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; 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; *olen = 0;
if( ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE ) {
return; return;
} }
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) );
if( (size_t)(end - p) < 5 ) if( end < p || (size_t)( end - p ) < 5 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -379,15 +379,16 @@ static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl,
unsigned char *p = buf; unsigned char *p = buf;
const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
*olen = 0;
if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED ) if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED )
{ {
*olen = 0;
return; return;
} }
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) );
if( (size_t)(end - p) < 4 ) if( end < p || (size_t)( end - p ) < 4 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -410,17 +411,18 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl,
unsigned char *p = buf; unsigned char *p = buf;
const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
*olen = 0;
if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED || if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED ||
ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 )
{ {
*olen = 0;
return; return;
} }
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac " MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac "
"extension" ) ); "extension" ) );
if( (size_t)(end - p) < 4 ) if( end < p || (size_t)( end - p ) < 4 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -443,17 +445,18 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl,
unsigned char *p = buf; unsigned char *p = buf;
const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
*olen = 0;
if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED || if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED ||
ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 )
{ {
*olen = 0;
return; return;
} }
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret " MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret "
"extension" ) ); "extension" ) );
if( (size_t)(end - p) < 4 ) if( end < p || (size_t)( end - p ) < 4 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -477,15 +480,16 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl,
const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN; const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
size_t tlen = ssl->session_negotiate->ticket_len; size_t tlen = ssl->session_negotiate->ticket_len;
*olen = 0;
if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED ) if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED )
{ {
*olen = 0;
return; return;
} }
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) );
if( (size_t)(end - p) < 4 + tlen ) if( end < p || (size_t)( end - p ) < 4 + tlen )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;
@ -521,9 +525,10 @@ static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl,
size_t alpnlen = 0; size_t alpnlen = 0;
const char **cur; const char **cur;
*olen = 0;
if( ssl->conf->alpn_list == NULL ) if( ssl->conf->alpn_list == NULL )
{ {
*olen = 0;
return; return;
} }
@ -532,7 +537,7 @@ static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl,
for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ )
alpnlen += *p = (unsigned char)( strlen( *cur ) & 0xFF ); alpnlen += *p = (unsigned char)( strlen( *cur ) & 0xFF );
if( (size_t)(end - p) < 6 + alpnlen ) if( end < p || (size_t)( end - p ) < 6 + alpnlen )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
return; return;