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.

1 files changed, 11 insertions, 2 deletions

diff --git a/lib/x509.c b/lib/x509.c
index 47347dfbf1..efdc1f37bd 100644
--- a/lib/x509.c
+++ b/lib/x509.c
@@ -1169,9 +1169,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 res->ncerts-1;
@@ -1387,9 +1390,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 res->ncerts-1;