Fix for memory leak when enumerating CRLs - bug #160635

4 files changed, 31 insertions, 58 deletions

diff --git a/security/nss/lib/softoken/pcert.h b/security/nss/lib/softoken/pcert.h
index c1d9b3128..14432ba58 100644
--- a/security/nss/lib/softoken/pcert.h
+++ b/security/nss/lib/softoken/pcert.h
@@ -68,9 +68,9 @@ nsslowcert_TraversePermCerts(NSSLOWCERTCertDBHandle *handle,
 PRBool
 nsslowcert_CertDBKeyConflict(SECItem *derCert, NSSLOWCERTCertDBHandle *handle);
 
-SECItem *
-nsslowcert_FindCrlByKey(NSSLOWCERTCertDBHandle *handle, SECItem *crlKey,
-				 		char **urlp, PRBool isKRL);
+certDBEntryRevocation *
+nsslowcert_FindCrlByKey(NSSLOWCERTCertDBHandle *handle,
+					 SECItem *crlKey, PRBool isKRL);
 
 SECStatus
 nsslowcert_DeletePermCRL(NSSLOWCERTCertDBHandle *handle,SECItem *derName,
diff --git a/security/nss/lib/softoken/pcertdb.c b/security/nss/lib/softoken/pcertdb.c
index bd50e00e1..dae11c2d6 100644
--- a/security/nss/lib/softoken/pcertdb.c
+++ b/security/nss/lib/softoken/pcertdb.c
@@ -4347,9 +4347,9 @@ nsslowcert_DestroyCertificateNoLocking(NSSLOWCERTCertificate *cert)
  * Lookup a CRL in the databases. We mirror the same fast caching data base
  *  caching stuff used by certificates....?
  */
-SECItem *
-nsslowcert_FindCrlByKey(NSSLOWCERTCertDBHandle *handle, SECItem *crlKey, 
-		char **url, PRBool isKRL)
+certDBEntryRevocation *
+nsslowcert_FindCrlByKey(NSSLOWCERTCertDBHandle *handle, 
+						SECItem *crlKey, PRBool isKRL)
 {
     SECItem keyitem;
     DBT key;
@@ -4380,20 +4380,12 @@ nsslowcert_FindCrlByKey(NSSLOWCERTCertDBHandle *handle, SECItem *crlKey,
 	goto loser;
     }
 
-    if (url && entry->url) {
-	*url = PORT_Strdup(entry->url);
-    }
-    crl = SECITEM_DupItem(&entry->derCrl);
-
 loser:
     if ( arena ) {
 	PORT_FreeArena(arena, PR_FALSE);
     }
-    if (entry) {
-	DestroyDBEntry((certDBEntry *)entry);
-    }
     
-    return(crl);
+    return entry;
 }
 
 /*
diff --git a/security/nss/lib/softoken/pkcs11.c b/security/nss/lib/softoken/pkcs11.c
index 2a6a43a85..c44448571 100644
--- a/security/nss/lib/softoken/pkcs11.c
+++ b/security/nss/lib/softoken/pkcs11.c
@@ -3545,12 +3545,13 @@ pk11_searchCrls(PK11Slot *slot, SECItem *derSubject, PRBool isKrl,
 	return;
     }
     if (derSubject->data != NULL)  {
-	SECItem *crl = 
-	    nsslowcert_FindCrlByKey(certHandle,derSubject,NULL,isKrl);
+	certDBEntryRevocation *crl = 
+	    nsslowcert_FindCrlByKey(certHandle, derSubject, isKrl);
 
 	if (crl != NULL) {
-	    pk11_addHandle(search, pk11_mkHandle(slot,derSubject,
+	    pk11_addHandle(search, pk11_mkHandle(slot, derSubject,
 		isKrl ? PK11_TOKEN_KRL_HANDLE : PK11_TOKEN_TYPE_CRL));
+	    nsslowcert_DestroyDBEntry((certDBEntry *)crl);
 	}
     } else {
 	pk11CrlData crlData;
diff --git a/security/nss/lib/softoken/pkcs11u.c b/security/nss/lib/softoken/pkcs11u.c
index 5e42975e0..314b09f7a 100644
--- a/security/nss/lib/softoken/pkcs11u.c
+++ b/security/nss/lib/softoken/pkcs11u.c
@@ -341,50 +341,27 @@ pk11_getSMime(PK11TokenObject *object)
     return entry;
 }
 
-static SECItem *
+static certDBEntryRevocation *
 pk11_getCrl(PK11TokenObject *object)
 {
-    SECItem *crl;
+    certDBEntryRevocation *crl;
     PRBool isKrl;
 
     if (object->obj.objclass != CKO_NETSCAPE_CRL) {
 	return NULL;
     }
     if (object->obj.objectInfo) {
-	return (SECItem *)object->obj.objectInfo;
+	return (certDBEntryRevocation *)object->obj.objectInfo;
     }
 
     isKrl = (PRBool) object->obj.handle == PK11_TOKEN_KRL_HANDLE;
-    crl = nsslowcert_FindCrlByKey(object->obj.slot->certDB,&object->dbKey,
-								NULL,isKrl);
+    crl = nsslowcert_FindCrlByKey(object->obj.slot->certDB,
+							&object->dbKey, isKrl);
     object->obj.objectInfo = (void *)crl;
-    object->obj.infoFree = (PK11Free) pk11_FreeItem;
+    object->obj.infoFree = (PK11Free) nsslowcert_DestroyDBEntry;
     return crl;
 }
 
-static char *
-pk11_getUrl(PK11TokenObject *object)
-{
-    SECItem *crl;
-    PRBool isKrl;
-    char *url = NULL;
-
-    if (object->obj.objclass != CKO_NETSCAPE_CRL) {
-	return NULL;
-    }
-
-    isKrl = (PRBool) object->obj.handle == PK11_TOKEN_KRL_HANDLE;
-    crl = nsslowcert_FindCrlByKey(object->obj.slot->certDB,&object->dbKey,
-								&url,isKrl);
-    if (object->obj.objectInfo == NULL) {
-	object->obj.objectInfo = (void *)crl;
-	object->obj.infoFree = (PK11Free) pk11_FreeItem;
-    } else {
-	if (crl) SECITEM_FreeItem(crl,PR_TRUE);
-    }
-    return url;
-}
-
 static NSSLOWCERTCertificate *
 pk11_getCert(PK11TokenObject *object)
 {
@@ -958,8 +935,7 @@ trust:
 static PK11Attribute *
 pk11_FindCrlAttribute(PK11TokenObject *object, CK_ATTRIBUTE_TYPE type)
 {
-    SECItem *crl;
-    char *url;
+    certDBEntryRevocation *crl;
 
     switch (type) {
     case CKA_PRIVATE:
@@ -968,19 +944,23 @@ pk11_FindCrlAttribute(PK11TokenObject *object, CK_ATTRIBUTE_TYPE type)
     case CKA_NETSCAPE_KRL:
 	return (PK11Attribute *) ((object->obj.handle == PK11_TOKEN_KRL_HANDLE) 
 			? &pk11_StaticTrueAttr : &pk11_StaticFalseAttr);
+    case CKA_SUBJECT:
+	return pk11_NewTokenAttribute(type,object->dbKey.data,
+						object->dbKey.len, PR_FALSE);	
+    default:
+	break;
+    }
+    crl =  pk11_getCrl(object);
+    switch (type) {
     case CKA_NETSCAPE_URL:
-	url = pk11_getUrl(object);
-	if (url == NULL) {
+	if (crl->url == NULL) {
 	    return (PK11Attribute *) &pk11_StaticNullAttr;
 	}
-	return pk11_NewTokenAttribute(type, url, PORT_Strlen(url)+1, PR_TRUE);
+	return pk11_NewTokenAttribute(type, crl->url, 
+					PORT_Strlen(crl->url)+1, PR_TRUE);
     case CKA_VALUE:
-	crl = pk11_getCrl(object);
-	if (crl == NULL) break;
-	return pk11_NewTokenAttribute(type, crl->data, crl->len, PR_FALSE);
-    case CKA_SUBJECT:
-	return pk11_NewTokenAttribute(type,object->dbKey.data,
-						object->dbKey.len, PR_FALSE);
+	return pk11_NewTokenAttribute(type, crl->derCrl.data, 
+						crl->derCrl.len, PR_FALSE);
     default:
 	break;
     }