From bb9dd0c044c3239aa9f82a6876b940ddc48a19ca Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 8 Jun 2017 11:55:34 +0100 Subject: [PATCH] Add hard assertion to mbedtls_ssl_read_record_layer This commit adds a hard assertion to mbedtls_ssl_read_record_layer triggering if both ssl->in_hslen and ssl->in_offt are not 0. This should never happen, and if it does, there's no sensible way of telling whether the previous message was a handshake or an application data message. --- library/ssl_tls.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 67cbdf423..911664eb9 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3777,6 +3777,7 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) * NOTE: This needs to be fixed, since like for * handshake messages it is allowed to have * multiple alerts witin a single record. + * Internal reference IOTSSL-1321. * * (3) Change cipher spec: * Consume whole record content, in_msglen = 0. @@ -3791,6 +3792,15 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) /* Case (1): Handshake messages */ if( ssl->in_hslen != 0 ) { + /* Hard assertion to be sure that no application data + * is in flight, as corrupting ssl->in_msglen during + * ssl->in_offt != NULL is fatal. */ + if( ssl->in_offt != NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + /* * Get next Handshake message in the current record */ @@ -3808,6 +3818,9 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) * Again, it's wrong for DTLS handshake fragmentation. * The following check is therefore mandatory, and * should not be treated as a silently corrected assertion. + * Additionally, ssl->in_hslen might be arbitrarily out of + * bounds after handling a DTLS message with an unexpected + * sequence number, see mbedtls_ssl_prepare_handshake_record. */ if( ssl->in_hslen < ssl->in_msglen ) {