From 52cd5ac21cdfdc0a6c016de97fe70d3a50baa526 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 4 Jul 2014 00:39:23 +0200
Subject: [PATCH] nss: fix a memory leak when CURLOPT_CRLFILE is used

---
 RELEASE-NOTES  |  1 +
 lib/urldata.h  |  1 +
 lib/vtls/nss.c | 38 +++++++++++++++++++++++++++++++++-----
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 5e7c3c13ea7452..5bdcb3caccad11 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -37,6 +37,7 @@ This release includes the following bugfixes:
  o nss: do not abort on connection failure (failing tests 305 and 404)
  o nss: make the fallback to SSLv3 work again
  o tool: prevent valgrind from reporting possibly lost memory (nss only)
+ o nss: fix a memory leak when CURLOPT_CRLFILE is used
  o 
 
 This release includes the following known bugs:
diff --git a/lib/urldata.h b/lib/urldata.h
index ebdad803d10622..5883dedbbd1d1c 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -324,6 +324,7 @@ struct ssl_connect_data {
   PRFileDesc *handle;
   char *client_nickname;
   struct SessionHandle *data;
+  struct curl_llist *crl_list;
   struct curl_llist *obj_list;
   PK11GenericObject *obj_clicert;
   ssl_connect_state connecting_state;
diff --git a/lib/vtls/nss.c b/lib/vtls/nss.c
index d22c9a9d9c103e..e858e5ff5af952 100644
--- a/lib/vtls/nss.c
+++ b/lib/vtls/nss.c
@@ -393,6 +393,14 @@ static void nss_destroy_object(void *user, void *ptr)
   PK11_DestroyGenericObject(obj);
 }
 
+/* same as nss_destroy_object() but for CRL items */
+static void nss_destroy_crl_item(void *user, void *ptr)
+{
+  SECItem *crl_der = (SECItem *)ptr;
+  (void) user;
+  SECITEM_FreeItem(crl_der, PR_TRUE);
+}
+
 static CURLcode nss_load_cert(struct ssl_connect_data *ssl,
                               const char *filename, PRBool cacert)
 {
@@ -431,7 +439,7 @@ static CURLcode nss_load_cert(struct ssl_connect_data *ssl,
 }
 
 /* add given CRL to cache if it is not already there */
-static CURLcode nss_cache_crl(SECItem *crl_der)
+static CURLcode nss_cache_crl(struct ssl_connect_data *ssl, SECItem *crl_der)
 {
   CERTCertDBHandle *db = CERT_GetDefaultCertDB();
   CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crl_der, 0);
@@ -442,12 +450,17 @@ static CURLcode nss_cache_crl(SECItem *crl_der)
     return CURLE_SSL_CRL_BADFILE;
   }
 
+  /* store the CRL item so that we can free it in Curl_nss_close() */
+  if(!Curl_llist_insert_next(ssl->crl_list, ssl->crl_list->tail, crl_der)) {
+    SECITEM_FreeItem(crl_der, PR_FALSE);
+    return CURLE_OUT_OF_MEMORY;
+  }
+
   /* acquire lock before call of CERT_CacheCRL() */
   PR_Lock(nss_crllock);
   if(SECSuccess != CERT_CacheCRL(db, crl_der)) {
     /* unable to cache CRL */
     PR_Unlock(nss_crllock);
-    SECITEM_FreeItem(crl_der, PR_TRUE);
     return CURLE_SSL_CRL_BADFILE;
   }
 
@@ -457,7 +470,8 @@ static CURLcode nss_cache_crl(SECItem *crl_der)
   return CURLE_OK;
 }
 
-static CURLcode nss_load_crl(const char* crlfilename)
+static CURLcode nss_load_crl(struct ssl_connect_data *connssl,
+                             const char* crlfilename)
 {
   PRFileDesc *infile;
   PRFileInfo  info;
@@ -512,7 +526,7 @@ static CURLcode nss_load_crl(const char* crlfilename)
     *crl_der = filedata;
 
   PR_Close(infile);
-  return nss_cache_crl(crl_der);
+  return nss_cache_crl(connssl, crl_der);
 
 fail:
   PR_Close(infile);
@@ -1213,6 +1227,10 @@ void Curl_nss_close(struct connectdata *conn, int sockindex)
     connssl->obj_list = NULL;
     connssl->obj_clicert = NULL;
 
+    /* destroy all CRL items */
+    Curl_llist_destroy(connssl->crl_list, NULL);
+    connssl->crl_list = NULL;
+
     PR_Close(connssl->handle);
     connssl->handle = NULL;
   }
@@ -1400,6 +1418,8 @@ static CURLcode nss_fail_connect(struct ssl_connect_data *connssl,
   /* cleanup on connection failure */
   Curl_llist_destroy(connssl->obj_list, NULL);
   connssl->obj_list = NULL;
+  Curl_llist_destroy(connssl->crl_list, NULL);
+  connssl->crl_list = NULL;
 
   if(connssl->handle
       && (SSL_VersionRangeGet(connssl->handle, &sslver) == SECSuccess)
@@ -1467,6 +1487,14 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)
   if(!connssl->obj_list)
     return CURLE_OUT_OF_MEMORY;
 
+  /* list of all CRL items we need to destroy in Curl_nss_close() */
+  connssl->crl_list = Curl_llist_alloc(nss_destroy_crl_item);
+  if(!connssl->crl_list) {
+    Curl_llist_destroy(connssl->obj_list, NULL);
+    connssl->obj_list = NULL;
+    return CURLE_OUT_OF_MEMORY;
+  }
+
   /* FIXME. NSS doesn't support multiple databases open at the same time. */
   PR_Lock(nss_initlock);
   curlerr = nss_init(conn->data);
@@ -1569,7 +1597,7 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)
   }
 
   if(data->set.ssl.CRLfile) {
-    const CURLcode rv = nss_load_crl(data->set.ssl.CRLfile);
+    const CURLcode rv = nss_load_crl(connssl, data->set.ssl.CRLfile);
     if(CURLE_OK != rv) {
       curlerr = rv;
       goto error;