bug 141355, CERT_DecodeDERCertificate is not a safe function for some uses, must be replaced with CERT_NewTempCertificate.

9 files changed, 77 insertions, 55 deletions

diff --git a/security/nss/lib/certdb/certdb.c b/security/nss/lib/certdb/certdb.c
index efe24325b..cfa5daa7d 100644
--- a/security/nss/lib/certdb/certdb.c
+++ b/security/nss/lib/certdb/certdb.c
@@ -1659,6 +1659,7 @@ CERT_IsCADERCert(SECItem *derCert, unsigned int *type) {
     CERTCertificate *cert;
     PRBool isCA;
 
+    /* This is okay -- only looks at extensions */
     cert = CERT_DecodeDERCertificate(derCert, PR_FALSE, NULL);
     if (cert == NULL) return PR_FALSE;
 
diff --git a/security/nss/lib/certdb/stanpcertdb.c b/security/nss/lib/certdb/stanpcertdb.c
index 357ae450d..78dbdaa0f 100644
--- a/security/nss/lib/certdb/stanpcertdb.c
+++ b/security/nss/lib/certdb/stanpcertdb.c
@@ -232,7 +232,12 @@ __CERT_NewTempCertificate(CERTCertDBHandle *handle, SECItem *derCert,
 	return NULL;
     }
     c->object = *pkio;
-    NSSITEM_FROM_SECITEM(&c->encoding, derCert);
+    if (copyDER) {
+	nssItem_Create(c->object.arena, &c->encoding, 
+	               derCert->len, derCert->data);
+    } else {
+	NSSITEM_FROM_SECITEM(&c->encoding, derCert);
+    }
     /* Forces a decoding of the cert in order to obtain the parts used
      * below
      */
@@ -583,7 +588,9 @@ CERT_DestroyCertificate(CERTCertificate *cert)
 	    }
 	    /* delete the NSSCertificate */
 	    NSSCertificate_Destroy(tmp);
-	} 
+	} else {
+	    PORT_FreeArena(cert->arena, PR_FALSE);
+	}
 #endif
     }
     return;
diff --git a/security/nss/lib/certhigh/certhigh.c b/security/nss/lib/certhigh/certhigh.c
index d4c432a48..4f6069ff2 100644
--- a/security/nss/lib/certhigh/certhigh.c
+++ b/security/nss/lib/certhigh/certhigh.c
@@ -854,6 +854,7 @@ cert_ImportCAChain(SECItem *certs, int numcerts, SECCertUsage certUsage, PRBool
 	certs++;
 
 	/* decode my certificate */
+	/* This use is ok -- only looks at decoded parts, calls NewTemp later */
 	newcert = CERT_DecodeDERCertificate(derCert, PR_FALSE, NULL);
 	if ( newcert == NULL ) {
 	    goto loser;
diff --git a/security/nss/lib/crmf/asn1cmn.c b/security/nss/lib/crmf/asn1cmn.c
index 59ce5b4a8..3ca062027 100644
--- a/security/nss/lib/crmf/asn1cmn.c
+++ b/security/nss/lib/crmf/asn1cmn.c
@@ -164,10 +164,7 @@ cmmf_DecodeDERCertificate(CERTCertDBHandle *db, SECItem *derCert)
 {
     CERTCertificate *newCert;
 
-    newCert = CERT_DecodeDERCertificate(derCert, PR_TRUE, NULL);
-    if (newCert != NULL && newCert->dbhandle == NULL) {
-        newCert->dbhandle = db;
-    }
+    newCert = CERT_NewTempCertificate(db, derCert, NULL, PR_FALSE, PR_TRUE);
     return newCert;
 }
 
diff --git a/security/nss/lib/crmf/respcmn.c b/security/nss/lib/crmf/respcmn.c
index d7c703ec0..871ece3ec 100644
--- a/security/nss/lib/crmf/respcmn.c
+++ b/security/nss/lib/crmf/respcmn.c
@@ -168,7 +168,8 @@ cmmf_MakeCertList(CERTCertificate **inCerts)
 	if (derCert->data == NULL) {
 	    derCert = freeCert = cmmf_encode_certificate(inCerts[i]);
 	}
-	currCert=CERT_DecodeDERCertificate(derCert, PR_TRUE, NULL);
+	currCert=CERT_NewTempCertificate(CERT_GetDefaultCertDB(), 
+	                                 derCert, NULL, PR_FALSE, PR_TRUE);
 	if (freeCert != NULL) {
 	    SECITEM_FreeItem(freeCert, PR_TRUE);
 	    freeCert = NULL;
diff --git a/security/nss/lib/pk11wrap/pk11cert.c b/security/nss/lib/pk11wrap/pk11cert.c
index c2c6a8e71..639d13956 100644
--- a/security/nss/lib/pk11wrap/pk11cert.c
+++ b/security/nss/lib/pk11wrap/pk11cert.c
@@ -58,11 +58,12 @@
 #include "pki3hack.h"
 #include "dev3hack.h"
 
-#include "dev.h" 
+#include "devm.h" 
 #include "nsspki.h"
 #include "pki.h"
 #include "pkim.h"
 #include "pkitm.h"
+#include "pkistore.h" /* to remove temp cert */
 
 #define PK11_SEARCH_CHUNKSIZE 10
 
@@ -391,59 +392,54 @@ pk11_isID0(PK11SlotInfo *slot, CK_OBJECT_HANDLE certID)
     return isZero;
 
 }
-     
+
+/*
+ * Create an NSSCertificate from a slot/certID pair, return it as a
+ * CERTCertificate.
+ */
 CERTCertificate
 *pk11_fastCert(PK11SlotInfo *slot, CK_OBJECT_HANDLE certID, 
 			CK_ATTRIBUTE *privateLabel, char **nickptr)
 {
-    CK_ATTRIBUTE certTemp[] = {
-	{ CKA_ID, NULL, 0 },
-	{ CKA_VALUE, NULL, 0 },
-	{ CKA_LABEL, NULL, 0 }
-    };
-    CK_ATTRIBUTE *id = &certTemp[0];
-    CK_ATTRIBUTE *certDER = &certTemp[1];
-    CK_ATTRIBUTE *label = &certTemp[2];
-    SECItem derCert;
-    int csize = sizeof(certTemp)/sizeof(certTemp[0]);
-    PRArenaPool *arena;
-    char *nickname;
-    CERTCertificate *cert;
-    CK_RV crv;
+    NSSCertificate *c;
+    nssCryptokiObject *co;
+    nssPKIObject *pkio;
+    NSSToken *token;
+    NSSTrustDomain *td = STAN_GetDefaultTrustDomain();
 
-    arena = PORT_NewArena( DER_DEFAULT_CHUNKSIZE);
-    if (arena == NULL) return NULL;
-    /*
-     * grab the der encoding
-     */
-    crv = PK11_GetAttributes(arena,slot,certID,certTemp,csize);
-    if (crv != CKR_OK) {
-	PORT_FreeArena(arena,PR_FALSE);
-	PORT_SetError( PK11_MapError(crv) );
+    /* Get the cryptoki object from the handle */
+    token = PK11Slot_GetNSSToken(slot);
+    co = nssCryptokiObject_Create(token, token->defaultSession, certID);
+    if (!co) {
 	return NULL;
     }
 
-    /*
-     * build a certificate out of it
-     */
-    derCert.data = (unsigned char*)certDER->pValue;
-    derCert.len = certDER->ulValueLen;
+    /* Create a PKI object from the cryptoki instance */
+    pkio = nssPKIObject_Create(NULL, co, td, NULL);
+    if (!pkio) {
+	nssCryptokiObject_Destroy(co);
+	return NULL;
+    }
 
-    /* figure out the nickname.... */
-    nickname = pk11_buildNickname(slot,label,privateLabel,id);
-    cert = CERT_DecodeDERCertificate(&derCert, PR_TRUE, nickname);
-    if (cert) {
-	cert->dbhandle = (CERTCertDBHandle *)
-			nssToken_GetTrustDomain(slot->nssToken);
+    /* Create a certificate */
+    c = nssCertificate_Create(pkio);
+    if (!c) {
+	nssPKIObject_Destroy(pkio);
+	return NULL;
     }
-	
+
+    /* Build the old-fashioned nickname */
     if (nickptr) {
-	*nickptr = nickname;
-    } else {
-	if (nickname) PORT_Free(nickname);
+	CK_ATTRIBUTE label, id;
+	label.type = CKA_LABEL;
+	label.pValue = co->label;
+	label.ulValueLen = PORT_Strlen(co->label);
+	id.type = CKA_ID;
+	id.pValue = c->id.data;
+	id.ulValueLen = c->id.size;
+	*nickptr = pk11_buildNickname(slot, &label, privateLabel, &id);
     }
-    PORT_FreeArena(arena,PR_FALSE);
-    return cert;
+    return STAN_GetCERTCertificate(c);
 }
 
 CK_TRUST
@@ -1700,6 +1696,14 @@ done:
 	c = STAN_GetNSSCertificate(cert);
     }
 
+    if (c->object.cryptoContext) {
+	/* Delete the temp instance */
+	nssCertificateStore_Remove(c->object.cryptoContext->certStore, c);
+	c->object.cryptoContext = NULL;
+	cert->istemp = PR_FALSE;
+	cert->isperm = PR_TRUE;
+    }
+
     /* set the id for the cert */
     nssItem_Create(c->object.arena, &c->id, keyID->len, keyID->data);
     if (!c->id.data) {
@@ -1885,6 +1889,9 @@ PK11_KeyForDERCertExists(SECItem *derCert, CK_OBJECT_HANDLE *keyPtr,
     CERTCertificate *cert;
     PK11SlotInfo *slot = NULL;
 
+    /* letting this use go -- the only thing that the cert is used for is
+     * to get the ID attribute.
+     */
     cert = CERT_DecodeDERCertificate(derCert, PR_FALSE, NULL);
     if (cert == NULL) return NULL;
 
@@ -1917,7 +1924,8 @@ PK11_ImportDERCertForKey(SECItem *derCert, char *nickname,void *wincx) {
     CERTCertificate *cert;
     PK11SlotInfo *slot = NULL;
 
-    cert = CERT_DecodeDERCertificate(derCert, PR_FALSE, NULL);
+    cert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+                                   derCert, NULL, PR_FALSE, PR_FALSE);
     if (cert == NULL) return NULL;
 
     slot = PK11_ImportCertForKey(cert, nickname, wincx);
diff --git a/security/nss/lib/pkcs12/p12d.c b/security/nss/lib/pkcs12/p12d.c
index 7e4eb56cd..0f0b1e671 100644
--- a/security/nss/lib/pkcs12/p12d.c
+++ b/security/nss/lib/pkcs12/p12d.c
@@ -2411,7 +2411,8 @@ sec_pkcs12_add_cert(sec_PKCS12SafeBag *cert, PRBool keyExists, void *wincx)
     if(keyExists) {
 	CERTCertificate *newCert;
 
-	newCert = CERT_DecodeDERCertificate( derCert, PR_FALSE, NULL);
+	newCert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+	                                  derCert, NULL, PR_FALSE, PR_FALSE);
 	if(!newCert) {
 	     if(nickName) SECITEM_ZfreeItem(nickName, PR_TRUE);
 	     cert->error = SEC_ERROR_NO_MEMORY;
@@ -2593,7 +2594,9 @@ SEC_PKCS12DecoderGetCerts(SEC_PKCS12DecoderContext *p12dcx)
 		CERTCertificate *tempCert = NULL;
 
 		if (derCert == NULL) continue;
-    		tempCert=CERT_DecodeDERCertificate(derCert, PR_TRUE, NULL);
+    		tempCert=CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+		                                 derCert, NULL, 
+		                                 PR_FALSE, PR_TRUE);
 
 		if (tempCert) {
 		    CERT_AddCertToListTail(certList,tempCert);
diff --git a/security/nss/lib/pkcs12/p12e.c b/security/nss/lib/pkcs12/p12e.c
index e3ddd08ce..0e393ed5e 100644
--- a/security/nss/lib/pkcs12/p12e.c
+++ b/security/nss/lib/pkcs12/p12e.c
@@ -1416,7 +1416,8 @@ SEC_PKCS12AddDERCertAndEncryptedKey(SEC_PKCS12ExportContext *p12ctxt,
 
     mark = PORT_ArenaMark(p12ctxt->arena);
 
-    cert = CERT_DecodeDERCertificate(derCert, PR_FALSE, NULL);
+    cert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+                                   derCert, NULL, PR_FALSE, PR_FALSE);
     if(!cert) {
 	PORT_ArenaRelease(p12ctxt->arena, mark);
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
diff --git a/security/nss/lib/pkcs7/certread.c b/security/nss/lib/pkcs7/certread.c
index 073a9e545..672e3b132 100644
--- a/security/nss/lib/pkcs7/certread.c
+++ b/security/nss/lib/pkcs7/certread.c
@@ -162,7 +162,8 @@ CERT_ConvertAndDecodeCertificate(char *certstr)
     if (rv != SECSuccess)
 	return NULL;
 
-    cert = CERT_DecodeDERCertificate(&der, PR_TRUE, NULL);
+    cert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(), 
+                                   &der, NULL, PR_FALSE, PR_TRUE);
 
     PORT_Free(der.data);
     return cert;
@@ -528,7 +529,9 @@ CERT_DecodeCertFromPackage(char *certbuf, int certlen)
     rv = CERT_DecodeCertPackage(certbuf, certlen, collect_certs,
 				(void *)&collectArgs);
     if ( rv == SECSuccess ) {
-	cert = CERT_DecodeDERCertificate(&collectArgs.cert, PR_TRUE, NULL);
+	cert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+	                               &collectArgs.cert, NULL, 
+	                               PR_FALSE, PR_TRUE);
     }
     
     PORT_FreeArena(collectArgs.arena, PR_FALSE);