pkcs11-helper icon indicating copy to clipboard operation
pkcs11-helper copied to clipboard

Avoid sementation fault in __pkcs11h_crypto_mbedtls_certificate_is_issuer

Open ko-maren opened this issue 1 year ago • 1 comments

I'm using pkcs11-helper v2.28.0 and mbedtls 3.6.0. There I'm observing a segmentation fault occurring in __pkcs11h_crypto_mbedtls_certificate_is_issuer. This segmentation fault comes from mbedtls (see https://github.com/Mbed-TLS/mbedtls/issues/9570), however it could prevented in pkcs11-helper as well/additional.

Instead of using the memset shortly before calling mbedtls_x509_crt_parse, the segmentation fault does not happened if defining x509_issuer and x509_cert at the beginning of the function.

Possible fix in pkcs11-helper:

static
int
__pkcs11h_crypto_mbedtls_certificate_is_issuer (
	IN void * const global_data,
	IN const unsigned char * const issuer_blob,
	IN const size_t issuer_blob_size,
	IN const unsigned char * const cert_blob,
	IN const size_t cert_blob_size
) {
	mbedtls_x509_crt x509_issuer;
	mbedtls_x509_crt x509_cert;
	memset(&x509_issuer, 0, sizeof(x509_issuer));
	memset(&x509_cert, 0, sizeof(x509_cert));

	uint32_t verify_flags = 0;
	PKCS11H_BOOL is_issuer = FALSE;

	(void)global_data;

	/*_PKCS11H_ASSERT (global_data!=NULL); NOT NEEDED*/
	_PKCS11H_ASSERT (issuer_blob!=NULL);
	_PKCS11H_ASSERT (cert_blob!=NULL);

	if (0 != mbedtls_x509_crt_parse (&x509_issuer, issuer_blob, issuer_blob_size)) {
		goto cleanup;
	}

	if (0 != mbedtls_x509_crt_parse (&x509_cert, cert_blob, cert_blob_size)) {
		goto cleanup;
	}

	if ( 0 == mbedtls_x509_crt_verify(&x509_cert, &x509_issuer, NULL, NULL,
		&verify_flags, NULL, NULL )) {
		is_issuer = TRUE;
	}

cleanup:
	mbedtls_x509_crt_free(&x509_cert);
	mbedtls_x509_crt_free(&x509_issuer);

	return is_issuer;
}

ko-maren avatar Sep 17 '24 09:09 ko-maren

Regarding of the response in the MBedTLS issue, mbedtls_x509_crt_init needs to be called before using x509_issuer and x509_cert. So this needs to be adjusted in __pkcs11h_crypto_mbedtls_certificate_is_issuer

ko-maren avatar Sep 17 '24 09:09 ko-maren

Do you have some simple code to demonstrate the crash?

saper avatar Oct 08 '24 15:10 saper

Hi @ko-maren, Somehow I missed this notification. Can you please test #70 ? Thanks, Alon

alonbl avatar Oct 09 '24 07:10 alonbl

@alonbl I also missed the notification. I will check it, and let you know. Thanks

ko-maren avatar Oct 22 '24 13:10 ko-maren

LGTM

ko-maren avatar Oct 28 '24 13:10 ko-maren

Thanks!

alonbl avatar Oct 28 '24 14:10 alonbl