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

5 files changed, 37 insertions, 15 deletions

diff --git a/src/lib/libcrypto/pkcs12/p12_add.c b/src/lib/libcrypto/pkcs12/p12_add.c
index 93c7c7221e..8ce1fede74 100644
--- a/src/lib/libcrypto/pkcs12/p12_add.c
+++ b/src/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/src/lib/libcrypto/pkcs12/p12_mutl.c b/src/lib/libcrypto/pkcs12/p12_mutl.c
index f0e6df9eb6..c71ed735ea 100644
--- a/src/lib/libcrypto/pkcs12/p12_mutl.c
+++ b/src/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/src/lib/libcrypto/pkcs12/pkcs12_local.h b/src/lib/libcrypto/pkcs12/pkcs12_local.h
index 1d6f0558ed..8d82d2f462 100644
--- a/src/lib/libcrypto/pkcs12/pkcs12_local.h
+++ b/src/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/src/lib/libcrypto/pkcs7/pk7_doit.c b/src/lib/libcrypto/pkcs7/pk7_doit.c
index 759d9dd5a5..ce0e99eec1 100644
--- a/src/lib/libcrypto/pkcs7/pk7_doit.c
+++ b/src/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/src/lib/libcrypto/pkcs7/pk7_mime.c b/src/lib/libcrypto/pkcs7/pk7_mime.c
index f00e18c7ef..381335589f 100644
--- a/src/lib/libcrypto/pkcs7/pk7_mime.c
+++ b/src/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;