397832 - libpkix leaks memory if a macro calls a function that returns an error. r=nelson
authoralexei.volkov.bugs%sun.com
Sat, 10 Nov 2007 01:45:04 +0000
changeset 8214 eeda617078d1807af3fb5bb308828ab57aa6f851
parent 8213 889f46fe6d147ce796e47c1528ba979424b54b6d
child 8215 95bc128ecbf27d6c2fb2df7500a463a73f8b7cf4
push idunknown
push userunknown
push dateunknown
reviewersnelson
bugs397832
397832 - libpkix leaks memory if a macro calls a function that returns an error. r=nelson
security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c
security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapdefaultclient.c
security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c
security/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_common.c
--- a/security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c
+++ b/security/nss/lib/libpkix/pkix/crlsel/pkix_crlselector.c
@@ -775,17 +775,17 @@ pkix_CRLSelector_Select(
 	PKIX_CRLSelector *selector,
 	PKIX_List *before,
 	PKIX_List **pAfter,
 	void *plContext)
 {
 	PKIX_Boolean match = PKIX_FALSE;
 	PKIX_UInt32 numBefore = 0;
 	PKIX_UInt32 i = 0;
-        PKIX_List *filtered = NULL;
+	PKIX_List *filtered = NULL;
 	PKIX_PL_CRL *candidate = NULL;
 
         PKIX_ENTER(CRLSELECTOR, "PKIX_CRLSelector_Select");
         PKIX_NULLCHECK_THREE(selector, before, pAfter);
 
         PKIX_CHECK(PKIX_List_Create(&filtered, plContext),
                 PKIX_LISTCREATEFAILED);
 
@@ -797,17 +797,17 @@ pkix_CRLSelector_Select(
                 PKIX_CHECK(PKIX_List_GetItem
                         (before, i, (PKIX_PL_Object **)&candidate, plContext),
                         PKIX_LISTGETITEMFAILED);
 
                 PKIX_CHECK_ONLY_FATAL(selector->matchCallback
                         (selector, candidate, &match, plContext),
                         PKIX_CRLSELECTORMATCHCALLBACKFAILED);
 
-                if ((!(PKIX_ERROR_RECEIVED)) && (match == PKIX_TRUE)) {
+                if (!(PKIX_ERROR_RECEIVED) && match == PKIX_TRUE) {
 
                         PKIX_CHECK_ONLY_FATAL(PKIX_List_AppendItem
                                 (filtered,
                                 (PKIX_PL_Object *)candidate,
                                 plContext),
                                 PKIX_LISTAPPENDITEMFAILED);
                 }
 
@@ -817,16 +817,18 @@ pkix_CRLSelector_Select(
 
         PKIX_CHECK(PKIX_List_SetImmutable(filtered, plContext),
                 PKIX_LISTSETIMMUTABLEFAILED);
 
         /* Don't throw away the list if one CRL was bad! */
         pkixTempErrorReceived = PKIX_FALSE;
 
         *pAfter = filtered;
+        filtered = NULL;
 
 cleanup:
 
+        PKIX_DECREF(filtered);
         PKIX_DECREF(candidate);
 
         PKIX_RETURN(CRLSELECTOR);
 
 }
--- 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
@@ -628,19 +628,17 @@ pkix_pl_LdapCertStore_GetCert(
         requestParams.timeLimit = 0;
 
         /* Prepare elements for request filter */
 
         /*
          * 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);
         }
 
         PKIX_CHECK(PKIX_CertSelector_GetCommonCertSelectorParams
                 (selector, &params, plContext),
                 PKIX_CERTSELECTORGETCOMCERTSELPARAMSFAILED);
 
@@ -678,16 +676,17 @@ pkix_pl_LdapCertStore_GetCert(
                                 PKIX_LISTCREATEFAILED);
 
                         PKIX_CHECK(PKIX_List_SetImmutable
                                 (filteredCerts, plContext),
                                 PKIX_LISTSETIMMUTABLEFAILED);
 
                         *pNBIOContext = NULL;
                         *pCertList = filteredCerts;
+                        filteredCerts = NULL;
                         goto cleanup;
                 }
         } else {
                 PKIX_ERROR(PKIX_INSUFFICIENTCRITERIAFORCERTQUERY);
         }
 
         /* Prepare attribute field of request */
 
@@ -745,23 +744,25 @@ pkix_pl_LdapCertStore_GetCert(
 
                 PKIX_CHECK(pkix_CertSelector_Select
                         (selector, unfilteredCerts, &filteredCerts, plContext),
                         PKIX_CERTSELECTORSELECTFAILED);
         }
 
         *pNBIOContext = NULL;
         *pCertList = filteredCerts;
+        filteredCerts = NULL;
 
 cleanup:
 
         PKIX_DECREF(params);
         PKIX_DECREF(subjectName);
         PKIX_DECREF(responses);
         PKIX_DECREF(unfilteredCerts);
+        PKIX_DECREF(filteredCerts);
         PKIX_DECREF(lcs);
 
         PKIX_RETURN(CERTSTORE);
 }
 
 /*
  * FUNCTION: pkix_pl_LdapCertStore_GetCertContinue
  *  (see description of PKIX_CertStore_CertCallback in pkix_certstore.h)
@@ -1001,16 +1002,17 @@ pkix_pl_LdapCertStore_GetCRL(
 
         /* Don't throw away the list if one CRL was bad! */
         pkixTempErrorReceived = PKIX_FALSE;
 
         *pNBIOContext = NULL;
         *pCrlList = filteredCRLs;
 
 cleanup:
+
         if (PKIX_ERROR_RECEIVED) {
                 PKIX_DECREF(filteredCRLs);
         }
 
         PKIX_DECREF(params);
         PKIX_DECREF(issuerNames);
         PKIX_DECREF(issuer);
         PKIX_DECREF(candidate);
--- 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
@@ -635,34 +635,32 @@ pkix_pl_LdapDefaultClient_CreateHelper(
         ldapDefaultClient->pollDesc.out_flags = 0;
 
         ldapDefaultClient->bindAPI = bindAPI;
 
         PKIX_CHECK(PKIX_PL_HashTable_Create
                 (LDAP_CACHEBUCKETS, 0, &ht, plContext),
                 PKIX_HASHTABLECREATEFAILED);
 
-        PKIX_INCREF(ht);
         ldapDefaultClient->cachePtr = ht;
 
         PKIX_CHECK(pkix_pl_Socket_GetCallbackList
                 (socket, &callbackList, plContext),
                 PKIX_SOCKETGETCALLBACKLISTFAILED);
 
         ldapDefaultClient->callbackList = callbackList;
 
         PKIX_INCREF(socket);
         ldapDefaultClient->clientSocket = socket;
 
         ldapDefaultClient->messageID = 0;
 
         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);
         }
         ldapDefaultClient->arena = arena;
 
         ldapDefaultClient->sendBuf = NULL;
         ldapDefaultClient->bytesToWrite = 0;
 
@@ -676,22 +674,21 @@ pkix_pl_LdapDefaultClient_CreateHelper(
 
         ldapDefaultClient->entriesFound = NULL;
         ldapDefaultClient->currentRequest = NULL;
         ldapDefaultClient->currentResponse = NULL;
 
         *pClient = ldapDefaultClient;
 
 cleanup:
+
         if (PKIX_ERROR_RECEIVED) {
                 PKIX_DECREF(ldapDefaultClient);
         }
 
-        PKIX_DECREF(ht);
-
         PKIX_RETURN(LDAPDEFAULTCLIENT);
 }
 
 /*
  * FUNCTION: PKIX_PL_LdapDefaultClient_Create
  * DESCRIPTION:
  *
  *  This function creates a new LdapDefaultClient using the PRNetAddr pointed to
--- 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
@@ -255,50 +255,49 @@ pkix_pl_Pk11CertStore_CertQuery(
                                 continue; /* just skip bad certs */
                         }
 
                         PKIX_CHECK_ONLY_FATAL(pkix_pl_Cert_CreateWithNSSCert
                                 (nssCert, &cert, plContext),
                                 PKIX_CERTCREATEWITHNSSCERTFAILED);
 
                         if (PKIX_ERROR_RECEIVED) {
+                                CERT_DestroyCertificate(nssCert);
+                                nssCert = NULL;
                                 continue; /* just skip bad certs */
                         }
 
                         PKIX_CHECK_ONLY_FATAL(PKIX_List_AppendItem
                                 (certList, (PKIX_PL_Object *)cert, plContext),
                                 PKIX_LISTAPPENDITEMFAILED);
 
                         PKIX_DECREF(cert);
 
                 }
 
                 /* Don't throw away the list if one cert was bad! */
                 pkixTempErrorReceived = PKIX_FALSE;
         }
 
         *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) {
+            CERT_DestroyCertList(pk11CertList);
         }
-
-        if (pk11CertList) {
-                PKIX_PL_NSSCALL
-                        (CERTSTORE, 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);
 }
 
 /*
  * FUNCTION: pkix_pl_Pk11CertStore_CrlQuery
  * DESCRIPTION:
  *
@@ -447,28 +446,26 @@ pkix_pl_Pk11CertStore_CrlQuery(
                 }
 
             }
         } else {
                 PKIX_ERROR(PKIX_INSUFFICIENTCRITERIAFORCRLQUERY);
         }
 
         *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);
         PKIX_DECREF(issuer);
         PKIX_DECREF(crl);
 
         PKIX_RETURN(CERTSTORE);
 }
@@ -569,21 +566,21 @@ pkix_pl_Pk11CertStore_GetCert(
 
         /* Don't throw away the list if one cert was bad! */
         pkixTempErrorReceived = PKIX_FALSE;
 
         PKIX_CHECK(PKIX_List_SetImmutable(filtered, plContext),
                 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);
 
         PKIX_RETURN(CERTSTORE);
 }
 
 /*
@@ -661,21 +658,21 @@ pkix_pl_Pk11CertStore_GetCRL(
 
         /* Don't throw away the list if one CRL was bad! */
         pkixTempErrorReceived = PKIX_FALSE;
 
         PKIX_CHECK(PKIX_List_SetImmutable(filtered, plContext),
                 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);
 
         PKIX_RETURN(CERTSTORE);
 }
 
 /* --Public-Pk11CertStore-Functions----------------------------------- */
--- a/security/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_common.c
+++ b/security/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_common.c
@@ -383,19 +383,21 @@ pkix_pl_helperBytes2Ascii(
                         (void) PL_strcat(outputString, ".");
                 }
         }
 
         /* Ensure output string ends with terminating null */
         outputString[outputLen-1] = '\0';
 
         *pAscii = outputString;
+        outputString = NULL;
 
 cleanup:
-
+        
+        PKIX_FREE(outputString);
         PKIX_FREE(tempString);
 
         PKIX_RETURN(OBJECT);
 
 }
 
 /*
  * FUNCTION: pkix_pl_ipAddrBytes2Ascii