From 287527042b5ddf563b263c56de865621b664586b Mon Sep 17 00:00:00 2001 From: Leonid Rozenboim Date: Thu, 21 Apr 2022 18:00:52 -0700 Subject: [PATCH 1/3] Avoid potentially passing NULL arguments Several call sites flagged by Coverity that may potentially cause a pointer argument to be NULL. In two cases the issue is using a function call as a parameter to a second function, where the first function may return NULL, while the second function does not check for the NULL argument value. Remaining case is when static configuration is mixed with run-time decision, that could result in a data buffer argument being NULL. Signed-off-by: Leonid Rozenboim --- library/ssl_tls12_server.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 486632ee8e..8e0ec6a56f 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2628,8 +2628,9 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if ( mbedtls_ssl_ciphersuite_uses_ec( - mbedtls_ssl_ciphersuite_from_id( ssl->session_negotiate->ciphersuite ) ) ) + const mbedtls_ssl_ciphersuite_t *suite = + mbedtls_ssl_ciphersuite_from_id( ssl->session_negotiate->ciphersuite ); + if ( suite != NULL && mbedtls_ssl_ciphersuite_uses_ec( suite) ) { ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); ext_len += olen; @@ -2854,7 +2855,14 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if( ! mbedtls_pk_can_do( mbedtls_ssl_own_key( ssl ), MBEDTLS_PK_ECKEY ) ) + const mbedtls_pk_context *private_key = mbedtls_ssl_own_key( ssl ); + if( private_key == NULL) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no ECDH private key" ) ); + return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); + } + + if( ! mbedtls_pk_can_do( private_key, MBEDTLS_PK_ECKEY ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key not ECDH capable" ) ); return( MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH ); @@ -3233,6 +3241,12 @@ curve_matching_done: */ if( md_alg != MBEDTLS_MD_NONE ) { + if( dig_signed == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + ret = mbedtls_ssl_get_key_exchange_md_tls1_2( ssl, hash, &hashlen, dig_signed, dig_signed_len, From dd428d36503c733b9386108f12bdbe3faadac3cd Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 13 May 2022 17:43:16 +0100 Subject: [PATCH 2/3] Fix incorrect error message Signed-off-by: Paul Elliott --- library/ssl_tls12_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 8e0ec6a56f..05a576778b 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2858,7 +2858,7 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) const mbedtls_pk_context *private_key = mbedtls_ssl_own_key( ssl ); if( private_key == NULL) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no ECDH private key" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no server private key" ) ); return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); } From 114203814a32125ef8ca6145ec15a3746404bf97 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 13 May 2022 17:43:47 +0100 Subject: [PATCH 3/3] Better check for NULL pointer Signed-off-by: Paul Elliott --- library/ssl_tls12_server.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 05a576778b..5aa541b44c 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -3203,6 +3203,12 @@ curve_matching_done: #if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) if( mbedtls_ssl_ciphersuite_uses_server_signature( ciphersuite_info ) ) { + if( dig_signed == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + size_t dig_signed_len = ssl->out_msg + ssl->out_msglen - dig_signed; size_t hashlen = 0; #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -3241,12 +3247,6 @@ curve_matching_done: */ if( md_alg != MBEDTLS_MD_NONE ) { - if( dig_signed == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - ret = mbedtls_ssl_get_key_exchange_md_tls1_2( ssl, hash, &hashlen, dig_signed, dig_signed_len,