# HG changeset patch
# User John M. Schanck <jschanck@mozilla.com>
# Date 1675974326 0
# Node ID 684586ec163ad4fbbf15ea2cd1ee5c2da43036ad
# Parent  58d7a8a55aea6a363bb8c7a9a7752739c4d32823
Bug 1804640 - improve handling of unknown PKCS#12 safe bag types. r=rrelyea

Differential Revision: https://phabricator.services.mozilla.com/D167443

Upstream-Status: Backport [https://hg.mozilla.org/projects/nss/rev/684586ec163ad4fbbf15ea2cd1ee5c2da43036ad]
CVE: CVE-2023-0767
Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
---
 nss/lib/pkcs12/p12d.c    | 23 +++++++++++++++--------
 nss/lib/pkcs12/p12t.h    |  1 +
 nss/lib/pkcs12/p12tmpl.c |  4 ++--
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/nss/lib/pkcs12/p12d.c b/nss/lib/pkcs12/p12d.c
index 676f33b..decddc5 100644
--- a/nss/lib/pkcs12/p12d.c
+++ b/nss/lib/pkcs12/p12d.c
@@ -337,31 +337,38 @@ sec_pkcs12_decoder_safe_bag_update(void *arg, const char *data,
     SEC_PKCS12DecoderContext *p12dcx;
     SECStatus rv;

-    /* make sure that we are not skipping the current safeBag,
-     * and that there are no errors.  If so, just return rather
-     * than continuing to process.
-     */
-    if (!safeContentsCtx || !safeContentsCtx->p12dcx ||
-        safeContentsCtx->p12dcx->error || safeContentsCtx->skipCurrentSafeBag) {
+    if (!safeContentsCtx || !safeContentsCtx->p12dcx || !safeContentsCtx->currentSafeBagA1Dcx) {
         return;
     }
     p12dcx = safeContentsCtx->p12dcx;

+    /* make sure that there are no errors and we are not skipping the current safeBag */
+    if (p12dcx->error || safeContentsCtx->skipCurrentSafeBag) {
+        goto loser;
+    }
+
     rv = SEC_ASN1DecoderUpdate(safeContentsCtx->currentSafeBagA1Dcx, data, len);
     if (rv != SECSuccess) {
         p12dcx->errorValue = PORT_GetError();
+        p12dcx->error = PR_TRUE;
+        goto loser;
+    }
+
+    /* The update may have set safeContentsCtx->skipCurrentSafeBag, and we
+     * may not get another opportunity to clean up the decoder context.
+     */
+    if (safeContentsCtx->skipCurrentSafeBag) {
         goto loser;
     }

     return;

 loser:
-    /* set the error, and finish the decoder context.  because there
+    /* Finish the decoder context. Because there
      * is not a way of returning an error message, it may be worth
      * while to do a check higher up and finish any decoding contexts
      * that are still open.
      */
-    p12dcx->error = PR_TRUE;
     SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagA1Dcx);
     safeContentsCtx->currentSafeBagA1Dcx = NULL;
     return;
diff --git a/nss/lib/pkcs12/p12t.h b/nss/lib/pkcs12/p12t.h
index b22f0dd..d449afd 100644
--- a/nss/lib/pkcs12/p12t.h
+++ b/nss/lib/pkcs12/p12t.h
@@ -73,6 +73,7 @@ struct sec_PKCS12SafeBagStr {
         sec_PKCS12CRLBag *crlBag;
         sec_PKCS12SecretBag *secretBag;
         sec_PKCS12SafeContents *safeContents;
+        SECItem *unknownBag;
     } safeBagContent;

     sec_PKCS12Attribute **attribs;
diff --git a/nss/lib/pkcs12/p12tmpl.c b/nss/lib/pkcs12/p12tmpl.c
index 7437cbc..b08384f 100644
--- a/nss/lib/pkcs12/p12tmpl.c
+++ b/nss/lib/pkcs12/p12tmpl.c
@@ -30,12 +30,12 @@ sec_pkcs12_choose_safe_bag_type(void *src_or_dest, PRBool encoding)

     oiddata = SECOID_FindOID(&safeBag->safeBagType);
     if (oiddata == NULL) {
-        return SEC_ASN1_GET(SEC_AnyTemplate);
+        return SEC_ASN1_GET(SEC_PointerToAnyTemplate);
     }

     switch (oiddata->offset) {
         default:
-            theTemplate = SEC_ASN1_GET(SEC_AnyTemplate);
+            theTemplate = SEC_ASN1_GET(SEC_PointerToAnyTemplate);
             break;
         case SEC_OID_PKCS12_V1_KEY_BAG_ID:
             theTemplate = SEC_ASN1_GET(SECKEY_PointerToPrivateKeyInfoTemplate);
--
2.34.1