From 7b97f03f09537fb488cbeda28767aa478618745c Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 10 Jun 2013 23:42:48 +0200 Subject: cert_stuff: avoid double free in the PKCS12 code In the pkcs12 code, we get a list of x509 records returned from PKCS12_parse but when iterating over the list and passing each to SSL_CTX_add_extra_chain_cert() we didn't also properly remove them from the "stack", which made them get freed twice (both in sk_X509_pop_free() and then later in SSL_CTX_free). This isn't really documented anywhere... Bug: http://curl.haxx.se/bug/view.cgi?id=1236 Reported-by: Nikaiw --- lib/ssluse.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/ssluse.c b/lib/ssluse.c index e9ae45ae0..1bb732785 100644 --- a/lib/ssluse.c +++ b/lib/ssluse.c @@ -464,11 +464,22 @@ int cert_stuff(struct connectdata *conn, /* Set Certificate Verification chain */ if(ca && sk_X509_num(ca)) { for(i = 0; i < sk_X509_num(ca); i++) { - if(!SSL_CTX_add_extra_chain_cert(ctx, sk_X509_value(ca, i))) { + /* + * Note that sk_X509_pop() is used below to make sure the cert is + * removed from the stack properly before getting passed to + * SSL_CTX_add_extra_chain_cert(). Previously we used + * sk_X509_value() instead, but then we'd clean it in the subsequent + * sk_X509_pop_free() call. + */ + X509 *x = sk_X509_pop(ca); + if(!SSL_CTX_add_extra_chain_cert(ctx, x)) { failf(data, "cannot add certificate to certificate chain"); goto fail; } - if(!SSL_CTX_add_client_CA(ctx, sk_X509_value(ca, i))) { + /* SSL_CTX_add_client_CA() seems to work with either sk_* function, + * presumably because it duplicates what we pass to it. + */ + if(!SSL_CTX_add_client_CA(ctx, x)) { failf(data, "cannot add certificate to client CA list"); goto fail; } -- cgit v1.2.3