bug 144309, return value of STAN_GetCERTCertificate not checked

r=wtc

5 files changed, 55 insertions, 9 deletions

diff --git a/security/nss/lib/certdb/stanpcertdb.c b/security/nss/lib/certdb/stanpcertdb.c
index 1ed2e8d23..654da08dd 100644
--- a/security/nss/lib/certdb/stanpcertdb.c
+++ b/security/nss/lib/certdb/stanpcertdb.c
@@ -181,6 +181,9 @@ __CERT_AddTempCertToPerm(CERTCertificate *cert, char *nickname,
     /* reset the CERTCertificate fields */
     cert->nssCertificate = NULL;
     cert = STAN_GetCERTCertificate(c); /* will return same pointer */
+    if (!cert) {
+        return SECFailure;
+    }
     cert->istemp = PR_FALSE;
     cert->isperm = PR_TRUE;
     if (!trust) {
@@ -243,6 +246,9 @@ __CERT_NewTempCertificate(CERTCertDBHandle *handle, SECItem *derCert,
      * below
      */
     cc = STAN_GetCERTCertificate(c);
+    if (!cc) {
+        return NULL;
+    }
     nssItem_Create(c->object.arena, 
                    &c->issuer, cc->derIssuer.len, cc->derIssuer.data);
     nssItem_Create(c->object.arena, 
@@ -286,6 +292,9 @@ __CERT_NewTempCertificate(CERTCertDBHandle *handle, SECItem *derCert,
 	/* and use the "official" entry */
 	c = tempCert;
 	cc = STAN_GetCERTCertificate(c);
+        if (!cc) {
+            return NULL;
+        }
     } else {
 	return NULL;
     }
@@ -354,10 +363,16 @@ CERT_FindCertByName(CERTCertDBHandle *handle, SECItem *name)
     c = get_best_temp_or_perm(ct, cp);
     if (ct) {
 	CERTCertificate *cert = STAN_GetCERTCertificate(ct);
+        if (!cert) {
+            return NULL;
+        }
 	CERT_DestroyCertificate(cert);
     }
     if (cp) {
 	CERTCertificate *cert = STAN_GetCERTCertificate(cp);
+        if (!cert) {
+            return NULL;
+        }
 	CERT_DestroyCertificate(cert);
     }
     if (c) {
@@ -404,6 +419,9 @@ CERT_FindCertByNickname(CERTCertDBHandle *handle, char *nickname)
 	CERT_DestroyCertificate(cert);
 	if (ct) {
 	    CERTCertificate *cert2 = STAN_GetCERTCertificate(ct);
+            if (!cert2) {
+                return NULL;
+            }
 	    CERT_DestroyCertificate(cert2);
 	}
     } else {
@@ -454,6 +472,9 @@ CERT_FindCertByNicknameOrEmailAddr(CERTCertDBHandle *handle, char *name)
 	CERT_DestroyCertificate(cert);
 	if (ct) {
 	    CERTCertificate *cert2 = STAN_GetCERTCertificate(ct);
+            if (!cert2) {
+                return NULL;
+            }
 	    CERT_DestroyCertificate(cert2);
 	}
     } else {
@@ -519,14 +540,18 @@ CERT_CreateSubjectCertList(CERTCertList *certList, CERTCertDBHandle *handle,
     ci = tSubjectCerts;
     while (ci && *ci) {
 	cert = STAN_GetCERTCertificate(*ci);
-	add_to_subject_list(certList, cert, validOnly, sorttime);
+        if (cert) {
+	    add_to_subject_list(certList, cert, validOnly, sorttime);
+        }
 	ci++;
     }
     /* Iterate over the matching perm certs.  Add them to the list */
     ci = pSubjectCerts;
     while (ci && *ci) {
 	cert = STAN_GetCERTCertificate(*ci);
-	add_to_subject_list(certList, cert, validOnly, sorttime);
+        if (cert) {
+	    add_to_subject_list(certList, cert, validOnly, sorttime);
+        }
 	ci++;
     }
     nss_ZFreeIf(tSubjectCerts);
diff --git a/security/nss/lib/certhigh/certhigh.c b/security/nss/lib/certhigh/certhigh.c
index 4f6069ff2..1cd3916ea 100644
--- a/security/nss/lib/certhigh/certhigh.c
+++ b/security/nss/lib/certhigh/certhigh.c
@@ -1130,6 +1130,9 @@ loser:
     while (stanCert) {
 	SECItem derCert;
 	CERTCertificate *cCert = STAN_GetCERTCertificate(stanCert);
+	if (!cCert) {
+	    goto loser;
+	}
 	derCert.len = (unsigned int)stanCert->encoding.size;
 	derCert.data = (unsigned char *)stanCert->encoding.data;
 	SECITEM_CopyItem(arena, &chain->certs[i], &derCert);
@@ -1150,7 +1153,9 @@ loser:
     stanCert = stanChain[i];
     while (stanCert) {
 	CERTCertificate *cCert = STAN_GetCERTCertificate(stanCert);
-	CERT_DestroyCertificate(cCert);
+	if (cCert) {
+	    CERT_DestroyCertificate(cCert);
+	}
 	stanCert = stanChain[++i];
     }
     nss_ZFreeIf(stanChain);
diff --git a/security/nss/lib/pk11wrap/pk11cert.c b/security/nss/lib/pk11wrap/pk11cert.c
index fbc2ff882..ae0629d49 100644
--- a/security/nss/lib/pk11wrap/pk11cert.c
+++ b/security/nss/lib/pk11wrap/pk11cert.c
@@ -1200,6 +1200,7 @@ transfer_token_certs_to_collection(nssList *certList, NSSToken *token,
 	    }
 	    nssTokenArray_Destroy(tokens);
 	}
+	/* *must* be a valid CERTCertificate, came from cache */
 	CERT_DestroyCertificate(STAN_GetCERTCertificate(certs[i]));
     }
     nss_ZFreeIf(certs);
@@ -1426,7 +1427,10 @@ PK11_FindCertsFromNickname(char *nickname, void *wincx) {
     if (foundCerts) {
 	certList = CERT_NewCertList();
 	for (i=0, c = *foundCerts; c; c = foundCerts[++i]) {
-	    CERT_AddCertToListTail(certList, STAN_GetCERTCertificate(c));
+	    CERTCertificate *certCert = STAN_GetCERTCertificate(c);
+	    if (certCert) {
+		CERT_AddCertToListTail(certList, certCert);
+	    }
 	}
 	if (CERT_LIST_HEAD(certList) == NULL) {
 	    CERT_DestroyCertList(certList);
diff --git a/security/nss/lib/pki/certificate.c b/security/nss/lib/pki/certificate.c
index a7041830a..913fe559d 100644
--- a/security/nss/lib/pki/certificate.c
+++ b/security/nss/lib/pki/certificate.c
@@ -814,7 +814,9 @@ nssBestCertificate_Callback
 	     * what the trust values are for the cert.
 	     * Ignore the returned pointer, the refcount is in c anyway.
 	     */
-	    (void)STAN_GetCERTCertificate(c);
+	    if (STAN_GetCERTCertificate(c) == NULL) {
+		return PR_FAILURE;
+	    }
 #endif
 	    if (dc->matchUsage(dc, best->usage)) {
 		best->cert = nssCertificate_AddRef(c);
diff --git a/security/nss/lib/pki/pkibase.c b/security/nss/lib/pki/pkibase.c
index 8b7e95a2b..8479df315 100644
--- a/security/nss/lib/pki/pkibase.c
+++ b/security/nss/lib/pki/pkibase.c
@@ -360,7 +360,9 @@ nssCertificateArray_Destroy
 #ifdef NSS_3_4_CODE
 	    if ((*certp)->decoding) {
 		CERTCertificate *cc = STAN_GetCERTCertificate(*certp);
-		CERT_DestroyCertificate(cc);
+		if (cc) {
+		    CERT_DestroyCertificate(cc);
+		}
 		continue;
 	    }
 #endif
@@ -906,6 +908,9 @@ nssPKIObjectCollection_AddInstanceAsObject
     }
     if (!node->haveObject) {
 	node->object = (*collection->createObject)(node->object);
+	if (!node->object) {
+	    return PR_FAILURE;
+	}
 	node->haveObject = PR_TRUE;
     }
 #ifdef NSS_3_4_CODE
@@ -932,8 +937,10 @@ cert_destroyObject(nssPKIObject *o)
 #ifdef NSS_3_4_CODE
     if (c->decoding) {
 	CERTCertificate *cc = STAN_GetCERTCertificate(c);
-	CERT_DestroyCertificate(cc);
-	return;
+	if (cc) {
+	    CERT_DestroyCertificate(cc);
+	    return;
+	} /* else destroy it as NSSCertificate below */
     }
 #endif
     nssCertificate_Destroy(c);
@@ -1002,7 +1009,10 @@ cert_createObject(nssPKIObject *o)
     NSSCertificate *cert;
     cert = nssCertificate_Create(o);
 #ifdef NSS_3_4_CODE
-    (void)STAN_GetCERTCertificate(cert);
+    if (STAN_GetCERTCertificate(cert) == NULL) {
+	nssCertificate_Destroy(cert);
+	return (nssPKIObject *)NULL;
+    }
     /* In 3.4, have to maintain uniqueness of cert pointers by caching all
      * certs.  Cache the cert here, before returning.  If it is already
      * cached, take the cached entry.