bug 129370, temp cert trusts and S/MIME profiles may cause crashes

r/a=wtc

5 files changed, 112 insertions, 26 deletions

diff --git a/security/nss/lib/certdb/stanpcertdb.c b/security/nss/lib/certdb/stanpcertdb.c
index 20940b3e9..121268e37 100644
--- a/security/nss/lib/certdb/stanpcertdb.c
+++ b/security/nss/lib/certdb/stanpcertdb.c
@@ -695,13 +695,14 @@ CERT_SaveSMimeProfile(CERTCertificate *cert, SECItem *emailProfile,
     SECStatus rv = SECFailure;
     PRBool saveit;
     char *emailAddr;
-    SECItem oldprof;
+    SECItem oldprof, oldproftime;
     SECItem *oldProfile = NULL;
     SECItem *oldProfileTime = NULL;
     PK11SlotInfo *slot = NULL;
     NSSCertificate *c;
     NSSCryptoContext *cc;
     nssSMIMEProfile *stanProfile = NULL;
+    PRBool freeOldProfile = PR_FALSE;
     
     emailAddr = cert->emailAddr;
     
@@ -718,10 +719,13 @@ CERT_SaveSMimeProfile(CERTCertificate *cert, SECItem *emailProfile,
 	    PORT_Assert(stanProfile->profileData);
 	    SECITEM_FROM_NSSITEM(&oldprof, stanProfile->profileData);
 	    oldProfile = &oldprof;
+	    SECITEM_FROM_NSSITEM(&oldproftime, stanProfile->profileTime);
+	    oldProfileTime = &oldproftime;
 	}
     } else {
 	oldProfile = PK11_FindSMimeProfile(&slot, emailAddr, &cert->derSubject, 
 							&oldProfileTime); 
+	freeOldProfile = PR_TRUE;
     }
 
     saveit = PR_FALSE;
@@ -767,11 +771,18 @@ CERT_SaveSMimeProfile(CERTCertificate *cert, SECItem *emailProfile,
     if (saveit) {
 	if (cc) {
 	    if (stanProfile) {
-		/* well, it's hashed and in an arena, might as well just
-		 * overwrite the buffer
+		/* stanProfile is already stored in the crypto context,
+		 * overwrite the data
 		 */
-		NSSITEM_FROM_SECITEM(stanProfile->profileTime, profileTime);
-		NSSITEM_FROM_SECITEM(stanProfile->profileData, emailProfile);
+		NSSArena *arena = stanProfile->object.arena;
+		stanProfile->profileTime = nssItem_Create(arena, 
+		                                          NULL,
+		                                          profileTime->len,
+		                                          profileTime->data);
+		stanProfile->profileData = nssItem_Create(arena, 
+		                                          NULL,
+		                                          emailProfile->len,
+		                                          emailProfile->data);
 	    } else if (profileTime && emailProfile) {
 		PRStatus nssrv;
 		NSSDER subject;
@@ -804,12 +815,15 @@ CERT_SaveSMimeProfile(CERTCertificate *cert, SECItem *emailProfile,
     }
 
 loser:
-    if (oldProfile) {
+    if (oldProfile && freeOldProfile) {
     	SECITEM_FreeItem(oldProfile,PR_TRUE);
     }
-    if (oldProfileTime) {
+    if (oldProfileTime && freeOldProfile) {
     	SECITEM_FreeItem(oldProfileTime,PR_TRUE);
     }
+    if (stanProfile) {
+	nssSMIMEProfile_Destroy(stanProfile);
+    }
     
     return(rv);
 }
@@ -833,7 +847,7 @@ CERT_FindSMimeProfile(CERTCertificate *cert)
 	    if (rvItem) {
 		rvItem->data = stanProfile->profileData->data;
 	    }
-	    nssPKIObject_Destroy(&stanProfile->object);
+	    nssSMIMEProfile_Destroy(stanProfile);
 	}
 	return rvItem;
     }
diff --git a/security/nss/lib/pki/certificate.c b/security/nss/lib/pki/certificate.c
index 93db6807f..a584456b1 100644
--- a/security/nss/lib/pki/certificate.c
+++ b/security/nss/lib/pki/certificate.c
@@ -862,3 +862,51 @@ nssCertificateList_AddReferences
     (void)nssCertificateList_DoCallback(certList, add_ref_callback, NULL);
 }
 
+NSS_IMPLEMENT NSSTrust *
+nssTrust_AddRef
+(
+  NSSTrust *trust
+)
+{
+    if (trust) {
+	nssPKIObject_AddRef(&trust->object);
+    }
+    return trust;
+}
+
+NSS_IMPLEMENT PRStatus
+nssTrust_Destroy
+(
+  NSSTrust *trust
+)
+{
+    if (trust) {
+	(void)nssPKIObject_Destroy(&trust->object);
+    }
+    return PR_SUCCESS;
+}
+
+NSS_IMPLEMENT nssSMIMEProfile *
+nssSMIMEProfile_AddRef
+(
+  nssSMIMEProfile *profile
+)
+{
+    if (profile) {
+	nssPKIObject_AddRef(&profile->object);
+    }
+    return profile;
+}
+
+NSS_IMPLEMENT PRStatus
+nssSMIMEProfile_Destroy
+(
+  nssSMIMEProfile *profile
+)
+{
+    if (profile) {
+	(void)nssPKIObject_Destroy(&profile->object);
+    }
+    return PR_SUCCESS;
+}
+
diff --git a/security/nss/lib/pki/pki.h b/security/nss/lib/pki/pki.h
index a232498f7..06746b0e0 100644
--- a/security/nss/lib/pki/pki.h
+++ b/security/nss/lib/pki/pki.h
@@ -83,6 +83,30 @@ nssCryptoContext_FindSMIMEProfileForCertificate
   NSSCertificate *cert
 );
 
+NSS_EXTERN NSSTrust *
+nssTrust_AddRef
+(
+  NSSTrust *trust
+);
+
+NSS_EXTERN PRStatus
+nssTrust_Destroy
+(
+  NSSTrust *trust
+);
+
+NSS_EXTERN nssSMIMEProfile *
+nssSMIMEProfile_AddRef
+(
+  nssSMIMEProfile *profile
+);
+
+NSS_EXTERN PRStatus
+nssSMIMEProfile_Destroy
+(
+  nssSMIMEProfile *profile
+);
+
 NSS_EXTERN nssSMIMEProfile *
 nssSMIMEProfile_Create
 (
diff --git a/security/nss/lib/pki/pki3hack.c b/security/nss/lib/pki/pki3hack.c
index 351e1742e..dead887e8 100644
--- a/security/nss/lib/pki/pki3hack.c
+++ b/security/nss/lib/pki/pki3hack.c
@@ -777,7 +777,7 @@ nssTrust_GetCERTCertTrustForCert(NSSCertificate *c, CERTCertificate *cc)
 	         myTrustOrder < lastTrustOrder) {
 		t.codeSigning = tokenTrust->codeSigning;
 	    }
-	    (void)nssPKIObject_Destroy(&tokenTrust->object);
+	    (void)nssTrust_Destroy(tokenTrust);
 	    lastTrustOrder = myTrustOrder;
 	}
     }
@@ -856,7 +856,7 @@ fill_CERTCertificateFields(NSSCertificate *c, CERTCertificate *cc, PRBool forced
 	nssTrust = nssCryptoContext_FindTrustForCertificate(context, c);
 	if (nssTrust) {
 	    cc->trust = cert_trust_from_stan_trust(nssTrust, cc->arena);
-	    nssPKIObject_Destroy(&nssTrust->object);
+	    nssTrust_Destroy(nssTrust);
 	}
     } else if (instance) {
 	/* slot */
@@ -1047,15 +1047,13 @@ STAN_ChangeCertTrust(CERTCertificate *cc, CERTCertTrust *trust)
 	NSSCryptoContext *cc = c->object.cryptoContext;
 	nssrv = nssCryptoContext_ImportTrust(cc, nssTrust);
 	if (nssrv != PR_SUCCESS) {
-	    nssPKIObject_Destroy(&nssTrust->object);
+	    nssTrust_Destroy(nssTrust);
 	    return nssrv;
 	}
 	if (nssList_Count(c->object.instanceList) == 0) {
 	    /* The context is the only instance, finished */
 	    return nssrv;
 	}
-	/* prevent it from being destroyed */
-	nssPKIObject_AddRef(&nssTrust->object);
     }
     td = STAN_GetDefaultTrustDomain();
     if (PK11_IsReadOnly(cc->slot)) {
@@ -1087,7 +1085,7 @@ STAN_ChangeCertTrust(CERTCertificate *cc, CERTCertTrust *trust)
     } else {
 	nssrv = PR_FAILURE;
     }
-    (void)nssPKIObject_Destroy(&nssTrust->object);
+    (void)nssTrust_Destroy(nssTrust);
     return nssrv;
 }
 
diff --git a/security/nss/lib/pki/pkistore.c b/security/nss/lib/pki/pkistore.c
index ce55ff72e..39a99827a 100644
--- a/security/nss/lib/pki/pkistore.c
+++ b/security/nss/lib/pki/pkistore.c
@@ -276,10 +276,10 @@ remove_certificate_entry
     if (entry) {
 	nssHash_Remove(store->issuer_and_serial, cert);
 	if (entry->trust) {
-	    nssPKIObject_Destroy(&entry->trust->object);
+	    nssTrust_Destroy(entry->trust);
 	}
 	if (entry->profile) {
-	    nssPKIObject_Destroy(&entry->profile->object);
+	    nssSMIMEProfile_Destroy(entry->profile);
 	}
 	nss_ZFreeIf(entry);
     }
@@ -627,7 +627,7 @@ nssCertificateStore_AddTrust
     entry = (certificate_hash_entry *)
                               nssHash_Lookup(store->issuer_and_serial, cert);
     if (entry) {
-	entry->trust = trust;
+	entry->trust = nssTrust_AddRef(trust);
     }
     PZ_Unlock(store->lock);
     return (entry) ? PR_SUCCESS : PR_FAILURE;
@@ -641,14 +641,15 @@ nssCertificateStore_FindTrustForCertificate
 )
 {
     certificate_hash_entry *entry;
+    NSSTrust *rvTrust = NULL;
     PZ_Lock(store->lock);
     entry = (certificate_hash_entry *)
                               nssHash_Lookup(store->issuer_and_serial, cert);
-    PZ_Unlock(store->lock);
-    if (entry) {
-	return entry->trust;
+    if (entry && entry->trust) {
+	rvTrust = nssTrust_AddRef(entry->trust);
     }
-    return NULL;
+    PZ_Unlock(store->lock);
+    return rvTrust;
 }
 
 NSS_EXTERN PRStatus
@@ -665,7 +666,7 @@ nssCertificateStore_AddSMIMEProfile
     entry = (certificate_hash_entry *)
                               nssHash_Lookup(store->issuer_and_serial, cert);
     if (entry) {
-	entry->profile = profile;
+	entry->profile = nssSMIMEProfile_AddRef(profile);
     }
     PZ_Unlock(store->lock);
     return (entry) ? PR_SUCCESS : PR_FAILURE;
@@ -679,14 +680,15 @@ nssCertificateStore_FindSMIMEProfileForCertificate
 )
 {
     certificate_hash_entry *entry;
+    nssSMIMEProfile *rvProfile = NULL;
     PZ_Lock(store->lock);
     entry = (certificate_hash_entry *)
                               nssHash_Lookup(store->issuer_and_serial, cert);
-    PZ_Unlock(store->lock);
-    if (entry) {
-	return entry->profile;
+    if (entry && entry->profile) {
+	rvProfile = nssSMIMEProfile_AddRef(entry->profile);
     }
-    return NULL;
+    PZ_Unlock(store->lock);
+    return rvProfile;
 }
 
 /* XXX this is also used by cache and should be somewhere else */