From 10bba00b3a10330dd13082e24ecd559103baac15 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Mon, 12 Sep 2016 10:22:37 +0200 Subject: gnutls_certificate_set_*key: ensure proper cleanup on key mismatch failures That is, ensure that we keep no local references that are shared with the caller, and that we properly free all initialized values. --- lib/gnutls_x509.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/gnutls_x509.c b/lib/gnutls_x509.c index 10256cc6bf..6cfc70aef5 100644 --- a/lib/gnutls_x509.c +++ b/lib/gnutls_x509.c @@ -1085,9 +1085,12 @@ gnutls_certificate_set_x509_key(gnutls_certificate_credentials_t res, res->ncerts++; + /* after this point we do not deinitialize anything on failure to avoid + * double freeing. We intentionally keep everything as the credentials state + * is documented to be on undefined state. */ if ((ret = _gnutls_check_key_cert_match(res)) < 0) { gnutls_assert(); - goto cleanup; + return ret; } return 0; @@ -1300,9 +1303,15 @@ gnutls_certificate_set_key(gnutls_certificate_credentials_t res, res->ncerts++; + /* Unlike gnutls_certificate_set_x509_key, we deinitialize everything + * local after a failure. That is because the caller is responsible for + * freeing these values after a failure, and if we keep references we + * lead to double freeing */ if ((ret = _gnutls_check_key_cert_match(res)) < 0) { gnutls_assert(); - return ret; + gnutls_free(new_pcert_list); + res->ncerts--; + goto cleanup; } return 0; -- cgit v1.2.1