From d96e10bf234d0338393ab9bae26c80d496aa00a6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 9 Jul 2019 17:30:02 +0100 Subject: [PATCH] Check architectural bound for max record payload len in one place The previous code performed architectural maximum record length checks both before and after record decryption. Since MBEDTLS_SSL_IN_CONTENT_LEN bounds the maximum length of the record plaintext, it suffices to check only once after (potential) decryption. This must not be confused with the internal check that the record length is small enough to make the record fit into the internal input buffer; this is done in mbedtls_ssl_fetch_input(). --- library/ssl_tls.c | 47 ++++++++++------------------------------------- 1 file changed, 10 insertions(+), 37 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5f81939c4..9682bbbb1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5025,43 +5025,13 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) /* Check length against bounds of the current transform and version */ - if( ssl->transform_in == NULL ) - { - if( ssl->in_msglen > MBEDTLS_SSL_IN_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } - } - else + if( ssl->transform_in != NULL ) { if( ssl->in_msglen < ssl->transform_in->minlen ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } - -#if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && - ssl->in_msglen > ssl->transform_in->minlen + MBEDTLS_SSL_IN_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } -#endif -#if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ - defined(MBEDTLS_SSL_PROTO_TLS1_2) - /* - * TLS encrypted messages can have up to 256 bytes of padding - */ - if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 && - ssl->in_msglen > ssl->transform_in->minlen + - MBEDTLS_SSL_IN_CONTENT_LEN + 256 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } -#endif } return( 0 ); @@ -5166,12 +5136,7 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ - if( ssl->in_msglen > MBEDTLS_SSL_IN_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } - else if( ssl->in_msglen == 0 ) + if( ssl->in_msglen == 0 ) { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 @@ -5244,6 +5209,14 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) } #endif + /* Check actual (decrypted) record content length against + * configured maximum. */ + if( ssl->in_msglen > MBEDTLS_SSL_IN_CONTENT_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } + return( 0 ); }