diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 5e7c3c13e..5bdcb3cac 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 ebdad803d..5883dedbb 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 d22c9a9d9..e858e5ff5 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;