397832 - libpkix leaks memory if a macro calls a function that returns an error. patch 291756. r=nelson

23 files changed, 110 insertions, 62 deletions

diff --git a/security/nss/lib/libpkix/pkix/certsel/pkix_certselector.c b/security/nss/lib/libpkix/pkix/certsel/pkix_certselector.c
index 4171c4d12..634867c15 100755
--- a/security/nss/lib/libpkix/pkix/certsel/pkix_certselector.c
+++ b/security/nss/lib/libpkix/pkix/certsel/pkix_certselector.c
@@ -1727,7 +1727,7 @@ pkix_CertSelector_Select(
 	PKIX_Boolean match = PKIX_FALSE;
 	PKIX_UInt32 numBefore = 0;
 	PKIX_UInt32 i = 0;
-        PKIX_List *filtered = NULL;
+	PKIX_List *filtered = NULL;
 	PKIX_PL_Cert *candidate = NULL;
 
         PKIX_ENTER(CERTSELECTOR, "PKIX_CertSelector_Select");
@@ -1769,9 +1769,11 @@ pkix_CertSelector_Select(
         pkixTempErrorReceived = PKIX_FALSE;
 
         *pAfter = filtered;
+        filtered = NULL;
 
 cleanup:
 
+        PKIX_DECREF(filtered);
         PKIX_DECREF(candidate);
 
         PKIX_RETURN(CERTSELECTOR);
diff --git a/security/nss/lib/libpkix/pkix/checker/pkix_certchainchecker.c b/security/nss/lib/libpkix/pkix/checker/pkix_certchainchecker.c
index 7ffd3b886..0818a0566 100755
--- a/security/nss/lib/libpkix/pkix/checker/pkix_certchainchecker.c
+++ b/security/nss/lib/libpkix/pkix/checker/pkix_certchainchecker.c
@@ -209,8 +209,10 @@ PKIX_CertChainChecker_Create(
         checker->state = initialState;
 
         *pChecker = checker;
-
+        checker = NULL;
 cleanup:
+        
+        PKIX_DECREF(checker);
 
         PKIX_RETURN(CERTCHAINCHECKER);
 
diff --git a/security/nss/lib/libpkix/pkix/checker/pkix_defaultrevchecker.c b/security/nss/lib/libpkix/pkix/checker/pkix_defaultrevchecker.c
index bb439c6dd..82034984c 100644
--- a/security/nss/lib/libpkix/pkix/checker/pkix_defaultrevchecker.c
+++ b/security/nss/lib/libpkix/pkix/checker/pkix_defaultrevchecker.c
@@ -185,9 +185,12 @@ pkix_DefaultRevChecker_Create(
         revChecker->certsRemaining = certsRemaining;
 
         *pRevChecker = revChecker;
+        revChecker = NULL;
 
 cleanup:
 
+        PKIX_DECREF(revChecker);
+
         PKIX_RETURN(DEFAULTREVOCATIONCHECKER);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/checker/pkix_ocspchecker.c b/security/nss/lib/libpkix/pkix/checker/pkix_ocspchecker.c
index 1ddfea917..a39bfd3cc 100644
--- a/security/nss/lib/libpkix/pkix/checker/pkix_ocspchecker.c
+++ b/security/nss/lib/libpkix/pkix/checker/pkix_ocspchecker.c
@@ -295,9 +295,12 @@ pkix_OcspChecker_Create(
         checkerObject->nbioContext = NULL;
 
         *pChecker = checkerObject;
+        checkerObject = NULL;
 
 cleanup:
 
+        PKIX_DECREF(checkerObject);
+
         PKIX_RETURN(OCSPCHECKER);
 
 }
diff --git a/security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c b/security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c
index a68d28b4e..a81f4aca1 100755
--- a/security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c
+++ b/security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c
@@ -645,12 +645,11 @@ PKIX_CRLSelector_Create(
         selector->context = crlSelectorContext;
 
         *pSelector = selector;
+        selector = NULL;
 
 cleanup:
 
-        if (PKIX_ERROR_RECEIVED){
-                PKIX_DECREF(selector);
-        }
+        PKIX_DECREF(selector);
 
         PKIX_RETURN(CRLSELECTOR);
 }
diff --git a/security/nss/lib/libpkix/pkix/params/pkix_trustanchor.c b/security/nss/lib/libpkix/pkix/params/pkix_trustanchor.c
index ba5eed649..5a8507e46 100755
--- a/security/nss/lib/libpkix/pkix/params/pkix_trustanchor.c
+++ b/security/nss/lib/libpkix/pkix/params/pkix_trustanchor.c
@@ -448,9 +448,11 @@ PKIX_TrustAnchor_CreateWithNameKeyPair(
         anchor->nameConstraints = nameConstraints;
 
         *pAnchor = anchor;
-
+        anchor = NULL;
 cleanup:
 
+        PKIX_DECREF(anchor);
+
         PKIX_RETURN(TRUSTANCHOR);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/results/pkix_buildresult.c b/security/nss/lib/libpkix/pkix/results/pkix_buildresult.c
index 3527e2ccd..95683f16e 100755
--- a/security/nss/lib/libpkix/pkix/results/pkix_buildresult.c
+++ b/security/nss/lib/libpkix/pkix/results/pkix_buildresult.c
@@ -339,9 +339,12 @@ pkix_BuildResult_Create(
                      PKIX_LISTSETIMMUTABLEFAILED);
 
         *pResult = result;
+        result = NULL;
 
 cleanup:
 
+        PKIX_DECREF(result);
+
         PKIX_RETURN(BUILDRESULT);
 
 }
diff --git a/security/nss/lib/libpkix/pkix/results/pkix_policynode.c b/security/nss/lib/libpkix/pkix/results/pkix_policynode.c
index 6ea3f7b06..2a3e647eb 100755
--- a/security/nss/lib/libpkix/pkix/results/pkix_policynode.c
+++ b/security/nss/lib/libpkix/pkix/results/pkix_policynode.c
@@ -178,11 +178,11 @@ pkix_PolicyNode_Create(
         node->depth = 0;
 
         *pObject = node;
+        node = NULL;
 
 cleanup:
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(node);
-        }
+
+        PKIX_DECREF(node);
 
         PKIX_RETURN(CERTPOLICYNODE);
 }
diff --git a/security/nss/lib/libpkix/pkix/results/pkix_valresult.c b/security/nss/lib/libpkix/pkix/results/pkix_valresult.c
index 3df6b8f91..4d02c7e5b 100755
--- a/security/nss/lib/libpkix/pkix/results/pkix_valresult.c
+++ b/security/nss/lib/libpkix/pkix/results/pkix_valresult.c
@@ -392,9 +392,12 @@ pkix_ValidateResult_Create(
         result->policyTree = policyTree;
 
         *pResult = result;
+        result = NULL;
 
 cleanup:
 
+        PKIX_DECREF(result);
+
         PKIX_RETURN(VALIDATERESULT);
 
 }
diff --git a/security/nss/lib/libpkix/pkix/store/pkix_store.c b/security/nss/lib/libpkix/pkix/store/pkix_store.c
index 8636c0acc..d41675b69 100755
--- a/security/nss/lib/libpkix/pkix/store/pkix_store.c
+++ b/security/nss/lib/libpkix/pkix/store/pkix_store.c
@@ -245,9 +245,12 @@ PKIX_CertStore_Create(
         certStore->certStoreContext = certStoreContext;
 
         *pStore = certStore;
+        certStore = NULL;
 
 cleanup:
 
+        PKIX_DECREF(certStore);
+
         PKIX_RETURN(CERTSTORE);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/top/pkix_basicconstraintschecker.c b/security/nss/lib/libpkix/pkix/top/pkix_basicconstraintschecker.c
index cccee2d37..809d4be3b 100755
--- a/security/nss/lib/libpkix/pkix/top/pkix_basicconstraintschecker.c
+++ b/security/nss/lib/libpkix/pkix/top/pkix_basicconstraintschecker.c
@@ -163,12 +163,11 @@ pkix_BasicConstraintsCheckerState_Create(
                     PKIX_OIDCREATEFAILED);
 
         *pState = state;
+        state = NULL;
 
 cleanup:
 
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(state);
-        }
+        PKIX_DECREF(state);
 
         PKIX_RETURN(BASICCONSTRAINTSCHECKERSTATE);
 }
diff --git a/security/nss/lib/libpkix/pkix/top/pkix_defaultcrlchecker.c b/security/nss/lib/libpkix/pkix/top/pkix_defaultcrlchecker.c
index 479702880..3244a905c 100755
--- a/security/nss/lib/libpkix/pkix/top/pkix_defaultcrlchecker.c
+++ b/security/nss/lib/libpkix/pkix/top/pkix_defaultcrlchecker.c
@@ -226,9 +226,12 @@ pkix_DefaultCRLCheckerState_Create(
         state->numCrlStores = 0;
 
         *pCheckerState = state;
+        state = NULL;
 
 cleanup:
 
+        PKIX_DECREF(state);
+
         PKIX_RETURN(DEFAULTCRLCHECKERSTATE);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/top/pkix_nameconstraintschecker.c b/security/nss/lib/libpkix/pkix/top/pkix_nameconstraintschecker.c
index eacd57129..738d82508 100755
--- a/security/nss/lib/libpkix/pkix/top/pkix_nameconstraintschecker.c
+++ b/security/nss/lib/libpkix/pkix/top/pkix_nameconstraintschecker.c
@@ -173,9 +173,12 @@ pkix_NameConstraintsCheckerState_Create(
         state->certsRemaining = numCerts;
 
         *pCheckerState = state;
+        state = NULL;
 
 cleanup:
 
+        PKIX_DECREF(state);
+
         PKIX_RETURN(CERTNAMECONSTRAINTSCHECKERSTATE);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/top/pkix_policychecker.c b/security/nss/lib/libpkix/pkix/top/pkix_policychecker.c
index 7584f4901..633125c79 100755
--- a/security/nss/lib/libpkix/pkix/top/pkix_policychecker.c
+++ b/security/nss/lib/libpkix/pkix/top/pkix_policychecker.c
@@ -527,12 +527,11 @@ pkix_PolicyCheckerState_Create(
         checkerState->mappedPolicyOIDs = NULL;
 
         *pCheckerState = checkerState;
+        checkerState = NULL;
 
 cleanup:
 
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(checkerState);
-        }
+        PKIX_DECREF(checkerState);
 
         PKIX_DECREF(anyPolicyList);
 
diff --git a/security/nss/lib/libpkix/pkix/top/pkix_signaturechecker.c b/security/nss/lib/libpkix/pkix/top/pkix_signaturechecker.c
index e350e3ce1..b03ef432c 100755
--- a/security/nss/lib/libpkix/pkix/top/pkix_signaturechecker.c
+++ b/security/nss/lib/libpkix/pkix/top/pkix_signaturechecker.c
@@ -153,12 +153,6 @@ pkix_SignatureCheckerState_Create(
         PKIX_ENTER(SIGNATURECHECKERSTATE, "pkix_SignatureCheckerState_Create");
         PKIX_NULLCHECK_TWO(trustedPubKey, pCheckerState);
 
-        PKIX_CHECK(PKIX_PL_OID_Create
-                    (PKIX_CERTKEYUSAGE_OID,
-                    &keyUsageOID,
-                    plContext),
-                    PKIX_OIDCREATEFAILED);
-
         PKIX_CHECK(PKIX_PL_Object_Alloc
                     (PKIX_SIGNATURECHECKERSTATE_TYPE,
                     sizeof (pkix_SignatureCheckerState),
@@ -170,20 +164,27 @@ pkix_SignatureCheckerState_Create(
 
         state->prevCertCertSign = PKIX_TRUE;
         state->prevPublicKeyList = NULL;
+        state->certsRemaining = certsRemaining;
 
         PKIX_INCREF(trustedPubKey);
-
-        state->certsRemaining = certsRemaining;
         state->prevPublicKey = trustedPubKey;
+
+        PKIX_CHECK(PKIX_PL_OID_Create
+                    (PKIX_CERTKEYUSAGE_OID,
+                    &keyUsageOID,
+                    plContext),
+                    PKIX_OIDCREATEFAILED);
+
         state->keyUsageOID = keyUsageOID;
+        keyUsageOID = NULL;
 
         *pCheckerState = state;
+        state = NULL;
 
 cleanup:
 
-        if (PKIX_ERROR_RECEIVED){
-                PKIX_DECREF(keyUsageOID);
-        }
+        PKIX_DECREF(keyUsageOID);
+        PKIX_DECREF(state); 
 
         PKIX_RETURN(SIGNATURECHECKERSTATE);
 }
diff --git a/security/nss/lib/libpkix/pkix/top/pkix_targetcertchecker.c b/security/nss/lib/libpkix/pkix/top/pkix_targetcertchecker.c
index 5b9391dde..57486fa2c 100755
--- a/security/nss/lib/libpkix/pkix/top/pkix_targetcertchecker.c
+++ b/security/nss/lib/libpkix/pkix/top/pkix_targetcertchecker.c
@@ -217,28 +217,38 @@ pkix_TargetCertCheckerState_Create(
                 }
         }
 
+        state->certsRemaining = certsRemaining;
+        state->subjAltNameMatchAll = subjAltNameMatchAll;
+
         PKIX_INCREF(certSelector);
         state->certSelector = certSelector;
+
         state->pathToNameList = pathToNameList;
+        pathToNameList = NULL;
+
         state->extKeyUsageList = extKeyUsageList;
+        extKeyUsageList = NULL;
+
         state->subjAltNameList = subjAltNameList;
-        state->subjAltNameMatchAll = subjAltNameMatchAll;
-        state->certsRemaining = certsRemaining;
+        subjAltNameList = NULL;
+
         state->extKeyUsageOID = extKeyUsageOID;
+        extKeyUsageOID = NULL;
+
         state->subjAltNameOID = subjAltNameOID;
+        subjAltNameOID = NULL;
 
         *pState = state;
+        state = NULL;
 
 cleanup:
-
-        if (PKIX_ERROR_RECEIVED){
-                PKIX_DECREF(extKeyUsageOID);
-                PKIX_DECREF(subjAltNameOID);
-                PKIX_DECREF(pathToNameList);
-                PKIX_DECREF(extKeyUsageList);
-                PKIX_DECREF(subjAltNameList);
-                PKIX_DECREF(state);
-        }
+        
+        PKIX_DECREF(extKeyUsageOID);
+        PKIX_DECREF(subjAltNameOID);
+        PKIX_DECREF(pathToNameList);
+        PKIX_DECREF(extKeyUsageList);
+        PKIX_DECREF(subjAltNameList);
+        PKIX_DECREF(state);
 
         PKIX_DECREF(certSelectorParams);
 
diff --git a/security/nss/lib/libpkix/pkix/util/pkix_error.c b/security/nss/lib/libpkix/pkix/util/pkix_error.c
index deebca8c7..b868f731e 100755
--- a/security/nss/lib/libpkix/pkix/util/pkix_error.c
+++ b/security/nss/lib/libpkix/pkix/util/pkix_error.c
@@ -320,6 +320,7 @@ pkix_Error_ToString(
 
 cleanup:
 
+        PKIX_DECREF(desc);
         PKIX_DECREF(causeString);
         PKIX_DECREF(formatString);
         PKIX_DECREF(optCauseString);
@@ -446,7 +447,7 @@ PKIX_Error_Create(
             tempCause = tempCause->cause) {
                 /* If we detect a loop, throw a new error */
                 if (tempCause == error) {
-                        PKIX_THROW(ERROR, PKIX_LOOPOFERRORCAUSEDETECTED);
+                        PKIX_ERROR(PKIX_LOOPOFERRORCAUSEDETECTED);
                 }
         }
 
@@ -459,8 +460,12 @@ PKIX_Error_Create(
         error->errCode = errCode;
 
         *pError = error;
+        error = NULL;
 
 cleanup:
+        /* PKIX-XXX Fix for leak during error creation */
+        PKIX_DECREF(error);
+
         PKIX_RETURN(ERROR);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/util/pkix_logger.c b/security/nss/lib/libpkix/pkix/util/pkix_logger.c
index 603c0e37f..433a9c32f 100644
--- a/security/nss/lib/libpkix/pkix/util/pkix_logger.c
+++ b/security/nss/lib/libpkix/pkix/util/pkix_logger.c
@@ -625,9 +625,12 @@ PKIX_Logger_Create(
         logger->context = loggerContext;
 
         *pLogger = logger;
+        logger = NULL;
 
 cleanup:
 
+        PKIX_DECREF(logger);
+
         PKIX_RETURN(LOGGER);
 }
 
diff --git a/security/nss/lib/libpkix/pkix/util/pkix_tools.c b/security/nss/lib/libpkix/pkix/util/pkix_tools.c
index 01b3e9cb7..0f89b4958 100755
--- a/security/nss/lib/libpkix/pkix/util/pkix_tools.c
+++ b/security/nss/lib/libpkix/pkix/util/pkix_tools.c
@@ -100,7 +100,7 @@ pkix_IsCertSelfIssued(
         PKIX_PL_X500Name *subject = NULL;
         PKIX_PL_X500Name *issuer = NULL;
 
-        PKIX_ENTER(CERT, "pkix_isCertSelfIssued");
+        PKIX_ENTER(CERT, "pkix_IsCertSelfIssued");
         PKIX_NULLCHECK_TWO(cert, pSelfIssued);
 
         PKIX_CHECK(PKIX_PL_Cert_GetSubject(cert, &subject, plContext),
@@ -121,6 +121,7 @@ pkix_IsCertSelfIssued(
 cleanup:
         PKIX_DECREF(subject);
         PKIX_DECREF(issuer);
+
         PKIX_RETURN(CERT);
 }
 
diff --git a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_colcertstore.c b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_colcertstore.c
index 0e5f16333..2f6a55d50 100755
--- a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_colcertstore.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_colcertstore.c
@@ -183,8 +183,12 @@ pkix_pl_CollectionCertStoreContext_Create(
         colCertStoreContext->certList = NULL;
 
         *pColCertStoreContext = colCertStoreContext;
+        colCertStoreContext = NULL;
 
 cleanup:
+
+        PKIX_DECREF(colCertStoreContext);
+
         PKIX_RETURN(COLLECTIONCERTSTORECONTEXT);
 }
 
diff --git a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ekuchecker.c b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ekuchecker.c
index 18c097380..7361d5bbf 100755
--- a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ekuchecker.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ekuchecker.c
@@ -283,14 +283,13 @@ pkix_pl_EkuCheckerState_Create(
         state->requiredExtKeyUsage = requiredExtKeyUsage;
 
         *pState = state;
+        state = NULL;
 
 cleanup:
 
         PKIX_DECREF(certSelector);
 
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(state);
-        }
+        PKIX_DECREF(state);
 
         PKIX_RETURN(USERDEFINEDMODULES);
 }
diff --git a/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_crl.c b/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_crl.c
index d431442b1..dad3cc564 100644
--- a/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_crl.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_crl.c
@@ -864,25 +864,25 @@ pkix_pl_CRL_CreateToList(
         PKIX_ENTER(CRL, "pkix_pl_CRL_CreateToList");
         PKIX_NULLCHECK_TWO(derCrlItem, crlList);
 
-        PKIX_PL_NSSCALLRV(CRL, nssCrl, CERT_DecodeDERCrl,
-                (NULL, derCrlItem, SEC_CRL_TYPE));
+        nssCrl = CERT_DecodeDERCrl(NULL, derCrlItem, SEC_CRL_TYPE);
+        if (nssCrl == NULL) {
+            goto cleanup;
+        }
 
-        if (nssCrl) {
-                PKIX_CHECK_ONLY_FATAL(pkix_pl_CRL_CreateWithSignedCRL
-                        (nssCrl, &crl, plContext),
-                        PKIX_CRLCREATEWITHSIGNEDCRLFAILED);
-
-                /* skip bad crls and append good ones */
-                if (!PKIX_ERROR_RECEIVED) {
-                        PKIX_CHECK(PKIX_List_AppendItem
-                                (crlList, (PKIX_PL_Object *) crl, plContext),
-                                PKIX_LISTAPPENDITEMFAILED);
-                }
+        PKIX_CHECK(pkix_pl_CRL_CreateWithSignedCRL
+                   (nssCrl, &crl, plContext),
+                   PKIX_CRLCREATEWITHSIGNEDCRLFAILED);
+        
+        nssCrl = NULL;
 
-                PKIX_DECREF(crl);
+        PKIX_CHECK(PKIX_List_AppendItem
+                   (crlList, (PKIX_PL_Object *) crl, plContext),
+                   PKIX_LISTAPPENDITEMFAILED);
 
-        }
 cleanup:
+        if (nssCrl) {
+            SEC_DestroyCrl(nssCrl);
+        }
 
         PKIX_DECREF(crl);
 
diff --git a/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_publickey.c b/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_publickey.c
index f75b331b3..28a1fade5 100644
--- a/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_publickey.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_publickey.c
@@ -182,12 +182,13 @@ pkix_pl_PublicKey_Destroy(
 
         pubKey = (PKIX_PL_PublicKey *)object;
 
-        PKIX_NULLCHECK_ONE(pubKey->nssSPKI);
+        if (pubKey->nssSPKI) {
 
-        PKIX_CHECK(pkix_pl_DestroySPKI(pubKey->nssSPKI, plContext),
-                    PKIX_DESTROYSPKIFAILED);
-
-        PKIX_FREE(pubKey->nssSPKI);
+            PKIX_CHECK(pkix_pl_DestroySPKI(pubKey->nssSPKI, plContext),
+                       PKIX_DESTROYSPKIFAILED);
+            
+            PKIX_FREE(pubKey->nssSPKI);
+        }
 
 cleanup: