From: Dr. David von Oheimb <dev@ddvo.net>
Date: Wed, 9 Jul 2025 15:51:26 +0000 (+0200)
Subject: APPS/load_key_certs_crls(): prevent mem leaks on error w.r.t. any leftover credentials
X-Git-Tag: 4.0-PRE-CLANG-FORMAT-WEBKIT~93
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3aea6c37f68a2fda6d8def3fbc6013886cde7211;p=thirdparty%2Fopenssl.git

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 4e3b162ec83..6838a48b4ce 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)