Merge pull request #6184 from leorosen/ssl_tls_curve_group_id_null_protect
mbedtls_ssl_check_curve prevent potential NULL pointer dereferencing
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index c45a1b8..5143abf 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4925,7 +4925,14 @@
*/
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 */
@@ -6568,14 +6575,27 @@
/* 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 NULL" ) );
+ 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 */