bug 125796, make sure all CERTCertificates are destroyed as such, and that cache collisions when traversing are handled correctly (by destroying extra reference)

security/nss/lib/certdb/stanpcertdb.c

@@ -335,8 +335,14 @@ CERT_FindCertByName(CERTCertDBHandle *handle, SECItem *name)
     cp = NSSTrustDomain_FindBestCertificateBySubject(handle, &subject, 
                                                      NULL, &usage, NULL);
     c = get_best_temp_or_perm(ct, cp);
-    if (ct) NSSCertificate_Destroy(ct);
-    if (cp) NSSCertificate_Destroy(cp);
+    if (ct) {
+	CERTCertificate *cert = STAN_GetCERTCertificate(ct);
+	CERT_DestroyCertificate(cert);
+    }
+    if (cp) {
+	CERTCertificate *cert = STAN_GetCERTCertificate(cp);
+	CERT_DestroyCertificate(cert);
+    }
     if (c) {
 	return STAN_GetCERTCertificate(c);
     } else {
@@ -379,7 +385,10 @@ CERT_FindCertByNickname(CERTCertDBHandle *handle, char *nickname)
     if (cert) {
 	c = get_best_temp_or_perm(ct, STAN_GetNSSCertificate(cert));
 	CERT_DestroyCertificate(cert);
-	if (ct) NSSCertificate_Destroy(ct);
+	if (ct) {
+	    CERTCertificate *cert2 = STAN_GetCERTCertificate(ct);
+	    CERT_DestroyCertificate(cert2);
+	}
     } else {
 	c = ct;
     }
@@ -426,7 +435,10 @@ CERT_FindCertByNicknameOrEmailAddr(CERTCertDBHandle *handle, char *name)
     if (cert) {
 	c = get_best_temp_or_perm(ct, STAN_GetNSSCertificate(cert));
 	CERT_DestroyCertificate(cert);
-	if (ct) NSSCertificate_Destroy(ct);
+	if (ct) {
+	    CERTCertificate *cert2 = STAN_GetCERTCertificate(ct);
+	    CERT_DestroyCertificate(cert2);
+	}
     } else {
 	c = ct;
     }
@@ -536,8 +548,6 @@ CERT_DestroyCertificate(CERTCertificate *cert)
         }
 #else
 	if (tmp) {
-	    /* delete the NSSCertificate */
-	    PK11SlotInfo *slot = cert->slot;
 	    NSSTrustDomain *td = STAN_GetDefaultTrustDomain();
 	    refCount = (int)tmp->object.refCount;
 	    /* This is a hack.  For 3.4, there are persistent references
@@ -559,8 +569,8 @@ CERT_DestroyCertificate(CERTCertificate *cert)
 		} else {
 		    nssTrustDomain_RemoveCertFromCache(td, tmp);
 		}
-		refCount = (int)tmp->object.refCount;
 	    }
+	    /* delete the NSSCertificate */
 	    NSSCertificate_Destroy(tmp);
 	} 
 #endif

security/nss/lib/pk11wrap/pk11cert.c

@@ -116,13 +116,11 @@ static PRStatus convert_and_cache_cert(NSSCertificate *c, void *arg)
      * destroy the reference to the copy, the callback will use the reference
      * to the cached entry, and everyone should be happy.
      */
-    if (cp == c) {
-	/* However, if the call to add c to the cache was successful, cp is
-	 * now an extra copy within this function and needs to be destroyed.
-	 */
-	NSSCertificate_Destroy(cp);
-    }
     nssrv = convert_cert(c, arg);
+    /* This function owns a reference to the cert, either from the AddRef
+     * or by getting it from the cache.
+     */
+    CERT_DestroyCertificate(STAN_GetCERTCertificate(c));
     return nssrv;
 }
 
@@ -1245,7 +1243,7 @@ get_newest_cert(NSSCertificate *c, void *arg)
     dc = nssCertificate_GetDecoding(c);
     founddc = nssCertificate_GetDecoding(*cfound);
     if (!founddc->isNewerThan(founddc, dc)) {
-	NSSCertificate_Destroy(*cfound);
+	CERT_DestroyCertificate(STAN_GetCERTCertificate(*cfound));
 	*cfound = nssCertificate_AddRef(c);
     }
     return PR_SUCCESS;
@@ -2667,7 +2665,7 @@ filter_list_for_token_certs(nssList *certList, NSSToken *token)
 	if (!isToken) {
 	    /* safe since iterator is copied */
 	    nssList_Remove(certList, c);
-	    NSSCertificate_Destroy(c);
+	    CERT_DestroyCertificate(STAN_GetCERTCertificate(c));
 	}
     }
     nssListIterator_Finish(certs);

security/nss/lib/pki/trustdomain.c

@@ -32,7 +32,7 @@
  */
 
 #ifdef DEBUG
-static const char CVS_ID[] = "@(#) $RCSfile: trustdomain.c,v $ $Revision: 1.33 $ $Date: 2002-02-08 15:13:13 $ $Name:  $";
+static const char CVS_ID[] = "@(#) $RCSfile: trustdomain.c,v $ $Revision: 1.34 $ $Date: 2002-02-26 16:21:48 $ $Name:  $";
 #endif /* DEBUG */
 
 #ifndef NSSPKI_H
@@ -927,13 +927,11 @@ static PRStatus traverse_callback(NSSCertificate *c, void *arg)
      * destroy the reference to the copy, the callback will use the reference
      * to the cached entry, and everyone should be happy.
      */
-    if (cp == c) {
-	/* However, if the call to add c to the cache was successful, cp is
-	 * now an extra copy within this function and needs to be destroyed.
-	 */
-	NSSCertificate_Destroy(cp);
-    }
     nssrv = (*ta->callback)(c, ta->arg);
+    /* This function owns a reference to the cert, either from the AddRef
+     * or by getting it from the cache.
+     */
+    CERT_DestroyCertificate(STAN_GetCERTCertificate(c));
     return nssrv;
 }