From 19e59735662864be54cf1ed48b09e7b5655553e0 Mon Sep 17 00:00:00 2001
From: Leonid Rozenboim <leonid.rozenboim@oracle.com>
Date: Mon, 8 Aug 2022 16:52:38 -0700
Subject: [PATCH] mbedtls_ssl_check_curve prevent potential NULL pointer
 dereferencing

Avoid the shorthand practice of the form 'x = func(foo)->bar' which
exposes the code to NULL pointer de-referencing when the 'func()'
returns a NULL pointer.

The first chunk is for when the curve group code is not recognized by
the library, and is cleanly rejected if offered.

The second chunk addresses the unlikely case of an internal error:
if 'mbedtls_pk_can_do()' returns TRUE, it should rule out
'mbedtls_pk_ec()' returning a NULL, unless there is a regression.

Signed-off-by: Leonid Rozenboim <leonid.rozenboim@oracle.com>
---
 library/ssl_tls.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 19b8a4135..670e761fa 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4902,7 +4902,14 @@ int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls
  */
 int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id )
 {
-    uint16_t tls_id = mbedtls_ecp_curve_info_from_grp_id( grp_id )->tls_id;
+    const mbedtls_ecp_curve_info *grp_info =
+        mbedtls_ecp_curve_info_from_grp_id(grp_id);
+
+    if (grp_info == NULL)
+        return -1;
+
+    uint16_t tls_id = grp_info->tls_id;
+
     return mbedtls_ssl_check_curve_tls_id( ssl, tls_id );
 }
 #endif /* MBEDTLS_ECP_C */
@@ -6545,14 +6552,27 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl,
         /* If certificate uses an EC key, make sure the curve is OK.
          * This is a public key, so it can't be opaque, so can_do() is a good
          * enough check to ensure pk_ec() is safe to use here. */
-        if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) &&
-            mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 )
+        if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) )
         {
-            ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY;
+            /* and in the unlikely case the above assumption no longer holds
+             * we are making sure that pk_ec() here does not return a NULL
+             */
+            const mbedtls_ecp_keypair *ec = mbedtls_pk_ec( *pk );
+            if( ec == NULL )
+            {
+                MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_pk_ec() returned MULL"));
+                return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
+            }
 
-            MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) );
-            if( ret == 0 )
-                ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE;
+            if( mbedtls_ssl_check_curve( ssl, ec->grp.id ) != 0 )
+            {
+                ssl->session_negotiate->verify_result |=
+                    MBEDTLS_X509_BADCERT_BAD_KEY;
+
+                MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) );
+                if( ret == 0 )
+                    ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE;
+            }
         }
     }
 #endif /* MBEDTLS_ECP_C */