Fix cert leaks when building a chain. There are several client auth bugs filed, this may not be for any one in particular, but was found with tstclnt.

3 files changed, 23 insertions, 7 deletions

diff --git a/security/nss/lib/certhigh/certhigh.c b/security/nss/lib/certhigh/certhigh.c
index c11e66152..261ac9755 100644
--- a/security/nss/lib/certhigh/certhigh.c
+++ b/security/nss/lib/certhigh/certhigh.c
@@ -1084,21 +1084,23 @@ loser:
 
     arena = PORT_NewArena(4096);
     if (arena == NULL) {
-	/* XXX destroy certs in chain */
-	nss_ZFreeIf(stanChain);
+	goto loser;
     }
 
     chain = (CERTCertificateList *)PORT_ArenaAlloc(arena, 
                                                  sizeof(CERTCertificateList));
+    if (!chain) goto loser;
     chain->certs = (SECItem*)PORT_ArenaAlloc(arena, len * sizeof(SECItem));
+    if (!chain->certs) goto loser;
     i = 0;
     stanCert = stanChain[i];
     while (stanCert) {
 	SECItem derCert;
+	CERTCertificate *cCert = STAN_GetCERTCertificate(stanCert);
 	derCert.len = (unsigned int)stanCert->encoding.size;
 	derCert.data = (unsigned char *)stanCert->encoding.data;
 	SECITEM_CopyItem(arena, &chain->certs[i], &derCert);
-	/* XXX CERT_DestroyCertificate(node->cert); */
+	CERT_DestroyCertificate(cCert);
 	stanCert = stanChain[++i];
     }
     if ( !includeRoot && len > 1) {
@@ -1110,6 +1112,19 @@ loser:
     chain->arena = arena;
     nss_ZFreeIf(stanChain);
     return chain;
+loser:
+    i = 0;
+    stanCert = stanChain[i];
+    while (stanCert) {
+	CERTCertificate *cCert = STAN_GetCERTCertificate(stanCert);
+	CERT_DestroyCertificate(cCert);
+	stanCert = stanChain[++i];
+    }
+    nss_ZFreeIf(stanChain);
+    if (arena) {
+	PORT_FreeArena(arena, PR_FALSE);
+    }
+    return NULL;
 #endif
 }
 
diff --git a/security/nss/lib/certhigh/certvfy.c b/security/nss/lib/certhigh/certvfy.c
index 61667bc4c..42d6d86ee 100644
--- a/security/nss/lib/certhigh/certvfy.c
+++ b/security/nss/lib/certhigh/certvfy.c
@@ -422,10 +422,11 @@ loser:
     if (status == PR_SUCCESS) {
 	/* if it's a root, the chain will only have one cert */
 	if (!chain[1]) {
-	    /* need to dupe since caller expects new cert */
-	    return CERT_DupCertificate(cert);
+	    /* already has a reference from the call to BuildChain */
+	    return cert;
 	} else {
-	    return STAN_GetCERTCertificate(chain[1]);
+	    CERT_DestroyCertificate(cert); /* the first cert in the chain */
+	    return STAN_GetCERTCertificate(chain[1]); /* return the 2nd */
 	}
     } else {
 	PORT_SetError (SEC_ERROR_UNKNOWN_ISSUER);
diff --git a/security/nss/lib/pki/certificate.c b/security/nss/lib/pki/certificate.c
index 1bfe1d7c8..868c08eaa 100644
--- a/security/nss/lib/pki/certificate.c
+++ b/security/nss/lib/pki/certificate.c
@@ -313,7 +313,7 @@ NSSCertificate_BuildChain
     }
 #endif
     chain = nssList_Create(NULL, PR_FALSE);
-    nssList_Add(chain, c);
+    nssList_Add(chain, nssCertificate_AddRef(c));
     if (statusOpt) *statusOpt = PR_SUCCESS;
     if (rvLimit == 1) goto finish;
     while (!nssItem_Equal(&c->subject, &c->issuer, &nssrv)) {