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

3 files changed, 26 insertions, 30 deletions

diff --git a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
index 3cb5bbc1f..3e01c2a03 100644
--- a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
@@ -633,9 +633,7 @@ pkix_pl_LdapCertStore_GetCert(
          * Get a short-lived arena. We'll be done with this space once
          * the request is encoded.
          */
-        PKIX_PL_NSSCALLRV
-            (CERTSTORE, requestArena, PORT_NewArena, (DER_DEFAULT_CHUNKSIZE));
-
+        requestArena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
         if (!requestArena) {
                 PKIX_ERROR_FATAL(PKIX_OUTOFMEMORY);
         }
@@ -683,6 +681,7 @@ pkix_pl_LdapCertStore_GetCert(
 
                         *pNBIOContext = NULL;
                         *pCertList = filteredCerts;
+                        filteredCerts = NULL;
                         goto cleanup;
                 }
         } else {
@@ -750,6 +749,7 @@ pkix_pl_LdapCertStore_GetCert(
 
         *pNBIOContext = NULL;
         *pCertList = filteredCerts;
+        filteredCerts = NULL;
 
 cleanup:
 
@@ -757,6 +757,7 @@ cleanup:
         PKIX_DECREF(subjectName);
         PKIX_DECREF(responses);
         PKIX_DECREF(unfilteredCerts);
+        PKIX_DECREF(filteredCerts);
         PKIX_DECREF(lcs);
 
         PKIX_RETURN(CERTSTORE);
@@ -1006,6 +1007,7 @@ pkix_pl_LdapCertStore_GetCRL(
         *pCrlList = filteredCRLs;
 
 cleanup:
+
         if (PKIX_ERROR_RECEIVED) {
                 PKIX_DECREF(filteredCRLs);
         }
diff --git a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapdefaultclient.c b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapdefaultclient.c
index db290429d..fff67eaa0 100644
--- a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapdefaultclient.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapdefaultclient.c
@@ -640,7 +640,6 @@ pkix_pl_LdapDefaultClient_CreateHelper(
                 (LDAP_CACHEBUCKETS, 0, &ht, plContext),
                 PKIX_HASHTABLECREATEFAILED);
 
-        PKIX_INCREF(ht);
         ldapDefaultClient->cachePtr = ht;
 
         PKIX_CHECK(pkix_pl_Socket_GetCallbackList
@@ -656,8 +655,7 @@ pkix_pl_LdapDefaultClient_CreateHelper(
 
         ldapDefaultClient->bindAPI = bindAPI;
 
-        PKIX_PL_NSSCALLRV
-            (LDAPDEFAULTCLIENT, arena, PORT_NewArena, (DER_DEFAULT_CHUNKSIZE));
+        arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
         if (!arena) {
             PKIX_ERROR_FATAL(PKIX_OUTOFMEMORY);
         }
@@ -681,12 +679,11 @@ pkix_pl_LdapDefaultClient_CreateHelper(
         *pClient = ldapDefaultClient;
 
 cleanup:
+
         if (PKIX_ERROR_RECEIVED) {
                 PKIX_DECREF(ldapDefaultClient);
         }
 
-        PKIX_DECREF(ht);
-
         PKIX_RETURN(LDAPDEFAULTCLIENT);
 }
 
diff --git a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c
index 5a535455d..0be59b1cf 100755
--- a/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c
@@ -260,6 +260,8 @@ pkix_pl_Pk11CertStore_CertQuery(
                                 PKIX_CERTCREATEWITHNSSCERTFAILED);
 
                         if (PKIX_ERROR_RECEIVED) {
+                                CERT_DestroyCertificate(nssCert);
+                                nssCert = NULL;
                                 continue; /* just skip bad certs */
                         }
 
@@ -276,24 +278,21 @@ pkix_pl_Pk11CertStore_CertQuery(
         }
 
         *pSelected = certList;
+        certList = NULL;
 
 cleanup:
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(certList);
-                if (arena) {
-                        PKIX_PL_NSSCALL
-                                (CERTSTORE, PORT_FreeArena, (arena, PR_FALSE));
-                }
-        }
-
+        
         if (pk11CertList) {
-                PKIX_PL_NSSCALL
-                        (CERTSTORE, CERT_DestroyCertList, (pk11CertList));
+            CERT_DestroyCertList(pk11CertList);
+        }
+        if (arena) {
+            PORT_FreeArena(arena, PR_FALSE);
         }
 
         PKIX_DECREF(subjectName);
         PKIX_DECREF(certValid);
         PKIX_DECREF(cert);
+        PKIX_DECREF(certList);
 
         PKIX_RETURN(CERTSTORE);
 }
@@ -452,18 +451,16 @@ pkix_pl_Pk11CertStore_CrlQuery(
         }
 
         *pSelected = crlList;
+        crlList = NULL;
 
 cleanup:
 
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(crlList);
-        }
+        PKIX_DECREF(crlList);
 
-        PKIX_PL_NSSCALL(CERTSTORE, ReleaseDPCache, (dpcache, writeLocked));
+        ReleaseDPCache(dpcache, writeLocked);
 
         if (arena) {
-                PKIX_PL_NSSCALL
-                        (CERTSTORE, PORT_FreeArena, (arena, PR_FALSE));
+            PORT_FreeArena(arena, PR_FALSE);
         }
 
         PKIX_DECREF(issuerNames);
@@ -574,11 +571,11 @@ pkix_pl_Pk11CertStore_GetCert(
                 PKIX_LISTSETIMMUTABLEFAILED);
 
         *pCertList = filtered;
+        filtered = NULL;
 
 cleanup:
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(filtered);
-        }
+
+        PKIX_DECREF(filtered);
         PKIX_DECREF(candidate);
         PKIX_DECREF(selected);
         PKIX_DECREF(params);
@@ -666,11 +663,11 @@ pkix_pl_Pk11CertStore_GetCRL(
                 PKIX_LISTSETIMMUTABLEFAILED);
 
         *pCrlList = filtered;
+        filtered = NULL;
 
 cleanup:
-        if (PKIX_ERROR_RECEIVED) {
-                PKIX_DECREF(filtered);
-        }
+
+        PKIX_DECREF(filtered);
         PKIX_DECREF(candidate);
         PKIX_DECREF(selected);
         PKIX_DECREF(params);