From: tb <tb@openbsd.org>
Date: Thu, 25 Jan 2024 13:44:08 +0000 (+0000)
Subject: Fix various NULL dereferences in PKCS #12
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=5fd44ab703ab05845f5818cf1f0c04c2b623b883;p=openbsd

Fix various NULL dereferences in PKCS #12

The PKCS #7 ContentInfo has a mandatory contentType, but the content itself
is OPTIONAL. Various unpacking API assumed presence of the content type is
enough to access members of the content, resulting in crashes.

Reported by Bahaa Naamneh on libressl-security, many thanks

ok jsing
---

diff --git a/lib/libcrypto/pkcs12/p12_add.c b/lib/libcrypto/pkcs12/p12_add.c
index 93c7c7221e1..8ce1fede74f 100644
--- a/lib/libcrypto/pkcs12/p12_add.c
+++ b/lib/libcrypto/pkcs12/p12_add.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: p12_add.c,v 1.22 2023/02/16 08:38:17 tb Exp $ */
+/* $OpenBSD: p12_add.c,v 1.23 2024/01/25 13:44:08 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -124,11 +124,15 @@ LCRYPTO_ALIAS(PKCS12_pack_p7data);
 STACK_OF(PKCS12_SAFEBAG) *
 PKCS12_unpack_p7data(PKCS7 *p7)
 {
+	ASN1_OCTET_STRING *aos;
+
 	if (!PKCS7_type_is_data(p7)) {
 		PKCS12error(PKCS12_R_CONTENT_TYPE_NOT_DATA);
 		return NULL;
 	}
-	return ASN1_item_unpack(p7->d.data, &PKCS12_SAFEBAGS_it);
+	if ((aos = PKCS7_get_octet_string(p7)) == NULL)
+		return NULL;
+	return ASN1_item_unpack(aos, &PKCS12_SAFEBAGS_it);
 }
 LCRYPTO_ALIAS(PKCS12_unpack_p7data);
 
@@ -182,11 +186,16 @@ LCRYPTO_ALIAS(PKCS12_pack_p7encdata);
 STACK_OF(PKCS12_SAFEBAG) *
 PKCS12_unpack_p7encdata(PKCS7 *p7, const char *pass, int passlen)
 {
+	PKCS7_ENC_CONTENT *content;
+
 	if (!PKCS7_type_is_encrypted(p7))
 		return NULL;
-	return PKCS12_item_decrypt_d2i(p7->d.encrypted->enc_data->algorithm,
-	    &PKCS12_SAFEBAGS_it, pass, passlen,
-	    p7->d.encrypted->enc_data->enc_data, 1);
+	if (p7->d.encrypted == NULL)
+		return NULL;
+	if ((content = p7->d.encrypted->enc_data) == NULL)
+		return NULL;
+	return PKCS12_item_decrypt_d2i(content->algorithm, &PKCS12_SAFEBAGS_it,
+	    pass, passlen, content->enc_data, 1);
 }
 LCRYPTO_ALIAS(PKCS12_unpack_p7encdata);
 
@@ -210,11 +219,14 @@ LCRYPTO_ALIAS(PKCS12_pack_authsafes);
 STACK_OF(PKCS7) *
 PKCS12_unpack_authsafes(const PKCS12 *p12)
 {
+	ASN1_OCTET_STRING *aos;
+
 	if (!PKCS7_type_is_data(p12->authsafes)) {
 		PKCS12error(PKCS12_R_CONTENT_TYPE_NOT_DATA);
 		return NULL;
 	}
-	return ASN1_item_unpack(p12->authsafes->d.data,
-	    &PKCS12_AUTHSAFES_it);
+	if ((aos = PKCS7_get_octet_string(p12->authsafes)) == NULL)
+		return NULL;
+	return ASN1_item_unpack(aos, &PKCS12_AUTHSAFES_it);
 }
 LCRYPTO_ALIAS(PKCS12_unpack_authsafes);
diff --git a/lib/libcrypto/pkcs12/p12_mutl.c b/lib/libcrypto/pkcs12/p12_mutl.c
index f0e6df9eb6e..c71ed735ea6 100644
--- a/lib/libcrypto/pkcs12/p12_mutl.c
+++ b/lib/libcrypto/pkcs12/p12_mutl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: p12_mutl.c,v 1.35 2023/02/16 08:38:17 tb Exp $ */
+/* $OpenBSD: p12_mutl.c,v 1.36 2024/01/25 13:44:08 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -115,6 +115,7 @@ PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
 {
 	const EVP_MD *md_type;
 	HMAC_CTX *hmac = NULL;
+	ASN1_OCTET_STRING *aos;
 	unsigned char key[EVP_MAX_MD_SIZE], *salt;
 	int saltlen, iter;
 	int md_size;
@@ -124,6 +125,10 @@ PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
 		PKCS12error(PKCS12_R_CONTENT_TYPE_NOT_DATA);
 		goto err;
 	}
+	if ((aos = PKCS7_get_octet_string(p12->authsafes)) == NULL) {
+		PKCS12error(PKCS12_R_DECODE_ERROR);
+		goto err;
+	}
 
 	salt = p12->mac->salt->data;
 	saltlen = p12->mac->salt->length;
@@ -155,8 +160,7 @@ PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
 		goto err;
 	if (!HMAC_Init_ex(hmac, key, md_size, md_type, NULL))
 		goto err;
-	if (!HMAC_Update(hmac, p12->authsafes->d.data->data,
-	    p12->authsafes->d.data->length))
+	if (!HMAC_Update(hmac, aos->data, aos->length))
 		goto err;
 	if (!HMAC_Final(hmac, mac, maclen))
 		goto err;
diff --git a/lib/libcrypto/pkcs12/pkcs12_local.h b/lib/libcrypto/pkcs12/pkcs12_local.h
index 1d6f0558ed5..8d82d2f4622 100644
--- a/lib/libcrypto/pkcs12/pkcs12_local.h
+++ b/lib/libcrypto/pkcs12/pkcs12_local.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: pkcs12_local.h,v 1.3 2022/11/26 17:23:18 tb Exp $ */
+/* $OpenBSD: pkcs12_local.h,v 1.4 2024/01/25 13:44:08 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -96,6 +96,9 @@ struct pkcs12_bag_st {
 	} value;
 };
 
+/* XXX - should go into pkcs7_local.h. */
+ASN1_OCTET_STRING *PKCS7_get_octet_string(PKCS7 *p7);
+
 __END_HIDDEN_DECLS
 
 #endif /* !HEADER_PKCS12_LOCAL_H */
diff --git a/lib/libcrypto/pkcs7/pk7_doit.c b/lib/libcrypto/pkcs7/pk7_doit.c
index 759d9dd5a52..ce0e99eec15 100644
--- a/lib/libcrypto/pkcs7/pk7_doit.c
+++ b/lib/libcrypto/pkcs7/pk7_doit.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: pk7_doit.c,v 1.54 2023/11/15 00:55:43 tb Exp $ */
+/* $OpenBSD: pk7_doit.c,v 1.55 2024/01/25 13:44:08 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -92,7 +92,7 @@ PKCS7_type_is_other(PKCS7* p7)
 
 }
 
-static ASN1_OCTET_STRING *
+ASN1_OCTET_STRING *
 PKCS7_get_octet_string(PKCS7 *p7)
 {
 	if (PKCS7_type_is_data(p7))
diff --git a/lib/libcrypto/pkcs7/pk7_mime.c b/lib/libcrypto/pkcs7/pk7_mime.c
index f00e18c7ef9..381335589f1 100644
--- a/lib/libcrypto/pkcs7/pk7_mime.c
+++ b/lib/libcrypto/pkcs7/pk7_mime.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: pk7_mime.c,v 1.19 2023/05/02 09:56:12 tb Exp $ */
+/* $OpenBSD: pk7_mime.c,v 1.20 2024/01/25 13:44:08 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -89,8 +89,11 @@ SMIME_write_PKCS7(BIO *bio, PKCS7 *p7, BIO *data, int flags)
 	STACK_OF(X509_ALGOR) *mdalgs = NULL;
 	int ctype_nid;
 
-	if ((ctype_nid = OBJ_obj2nid(p7->type)) == NID_pkcs7_signed)
+	if ((ctype_nid = OBJ_obj2nid(p7->type)) == NID_pkcs7_signed) {
+		if (p7->d.sign == NULL)
+			return 0;
 		mdalgs = p7->d.sign->md_algs;
+	}
 
 	flags ^= SMIME_OLDMIME;