Commit 3aea6c37f6 for openssl.org
commit 3aea6c37f68a2fda6d8def3fbc6013886cde7211
Author: Dr. David von Oheimb <dev@ddvo.net>
Date: Wed Jul 9 17:51:26 2025 +0200
APPS/load_key_certs_crls(): prevent mem leaks on error w.r.t. any leftover credentials
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28005)
diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 4e3b162ec8..6838a48b4c 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -732,11 +732,6 @@ int load_cert_certs(const char *uri,
warn_cert(uri, *pcert, 0, vpm);
if (pcerts != NULL)
warn_certs(uri, *pcerts, 1, vpm);
- } else {
- if (pcerts != NULL) {
- OSSL_STACK_OF_X509_free(*pcerts);
- *pcerts = NULL;
- }
}
return ret;
}
@@ -827,18 +822,10 @@ X509_STORE *load_certstore(char *input, const char *pass, const char *desc,
int load_certs(const char *uri, int maybe_stdin, STACK_OF(X509) **certs,
const char *pass, const char *desc)
{
- int ret, was_NULL = *certs == NULL;
-
if (desc == NULL)
desc = "certificates";
- ret = load_key_certs_crls(uri, FORMAT_UNDEF, maybe_stdin, pass, desc, 0,
- NULL, NULL, NULL, NULL, certs, NULL, NULL);
-
- if (!ret && was_NULL) {
- OSSL_STACK_OF_X509_free(*certs);
- *certs = NULL;
- }
- return ret;
+ return load_key_certs_crls(uri, FORMAT_UNDEF, maybe_stdin, pass, desc, 0,
+ NULL, NULL, NULL, NULL, certs, NULL, NULL);
}
/*
@@ -848,18 +835,10 @@ int load_certs(const char *uri, int maybe_stdin, STACK_OF(X509) **certs,
int load_crls(const char *uri, STACK_OF(X509_CRL) **crls,
const char *pass, const char *desc)
{
- int ret, was_NULL = *crls == NULL;
-
if (desc == NULL)
desc = "CRLs";
- ret = load_key_certs_crls(uri, FORMAT_UNDEF, 0, pass, desc, 0,
- NULL, NULL, NULL, NULL, NULL, NULL, crls);
-
- if (!ret && was_NULL) {
- sk_X509_CRL_pop_free(*crls, X509_CRL_free);
- *crls = NULL;
- }
- return ret;
+ return load_key_certs_crls(uri, FORMAT_UNDEF, 0, pass, desc, 0,
+ NULL, NULL, NULL, NULL, NULL, NULL, crls);
}
static const char *format2string(int format)
@@ -901,10 +880,9 @@ static const char *format2string(int format)
* If pcerts is non-NULL then all available certificates are appended to *pcerts
* except any certificate assigned to *pcert.
* If pcrls is non-NULL and *pcrls == NULL then a new list of CRLs is allocated.
- * If pcrls is non-NULL then all available CRLs are appended to *pcerts
+ * If pcrls is non-NULL then all available CRLs are appended to *pcrls
* except any CRL assigned to *pcrl.
- * In any case (also on error) the caller is responsible for freeing all members
- * of *pcerts and *pcrls (as far as they are not NULL).
+ * On error, any contents of non-NULL credential pointers are freed.
*/
int load_key_certs_crls(const char *uri, int format, int maybe_stdin,
const char *pass, const char *desc, int quiet,
@@ -1134,13 +1112,43 @@ int load_key_certs_crls(const char *uri, int format, int maybe_stdin,
}
BIO_printf(bio_err, "\n");
ERR_print_errors(bio_err);
- }
- if (quiet || failed == NULL)
+ ERR_clear_last_mark();
+ } else {
/* clear any suppressed or spurious errors */
ERR_pop_to_mark();
- else
- ERR_clear_last_mark();
- return failed == NULL;
+ }
+ if (failed != NULL) {
+ if (ppkey != NULL) {
+ EVP_PKEY_free(*ppkey);
+ *ppkey = NULL;
+ }
+ if (ppubkey != NULL) {
+ EVP_PKEY_free(*ppubkey);
+ *ppubkey = NULL;
+ }
+ if (pparams != NULL) {
+ EVP_PKEY_free(*pparams);
+ *pparams = NULL;
+ }
+ if (pcert != NULL) {
+ X509_free(*pcert);
+ *pcert = NULL;
+ }
+ if (pcerts != NULL) {
+ sk_X509_pop_free(*pcerts, X509_free);
+ *pcerts = NULL;
+ }
+ if (pcrl != NULL) {
+ X509_CRL_free(*pcrl);
+ *pcrl = NULL;
+ }
+ if (pcrls != NULL) {
+ sk_X509_CRL_pop_free(*pcrls, X509_CRL_free);
+ *pcrls = NULL;
+ }
+ return 0;
+ }
+ return 1;
}
#define X509V3_EXT_UNKNOWN_MASK (0xfL << 16)