Fix memory leak in load_key_certs_crls() when add/push fails

When X509_add_cert() or sk_X509_CRL_push() failed, the cert or CRL from
OSSL_STORE was not freed. Free on failure to avoid a leak.

Fix 90-test_memfail.t parsing of count output so the memfail suite runs
correctly: parse 'skip: N count M' with a regex (handles '# ' prefix),
return (0,0) if the count file cannot be opened, and skip with a clear
message when total malloc count is 0 instead of planning 0 tests.

Apply clang-format to test/load_key_certs_crls_memfail.c.

- apps/lib/apps.c: free cert/CRL on add/push failure
- test/build.info: add load_key_certs_crls_memfail (allocfail-tests)
- test/load_key_certs_crls_memfail.c: regression test for issue #30364
- test/recipes/90-test_memfail.t: fix get_count_info parsing and plan

Issue #30364

Fixes: 6d382c74b375 "Use OSSL_STORE for load_{,pub}key() and load_cert() in apps/lib/apps.c"
Fixes: d7fcee3b3b5fa "OSSL_HTTP_parse_url(): add optional port number return parameter and strengthen documentation"
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Tue Apr 21 08:50:18 2026
(Merged from https://github.com/openssl/openssl/pull/30428)

diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 986f5954f4a4936b907d1f85363ba41dad10983b..ce4fa9d38364b45ff33d3fe8ebeb2c1091656010 100644 (file)
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -1057,9 +1057,12 @@ int load_key_certs_crls(const char *uri, int format, int maybe_stdin,
                 if (ok)
                     pcert = NULL;
             } else if (pcerts != NULL) {
-                ok = X509_add_cert(*pcerts,
-                    OSSL_STORE_INFO_get1_CERT(info),
-                    X509_ADD_FLAG_DEFAULT);
+                X509 *cert = OSSL_STORE_INFO_get1_CERT(info);
+
+                ok = cert != NULL
+                    && X509_add_cert(*pcerts, cert, X509_ADD_FLAG_DEFAULT);
+                if (!ok)
+                    X509_free(cert);
             }
             ncerts += ok;
             break;
@@ -1069,7 +1072,11 @@ int load_key_certs_crls(const char *uri, int format, int maybe_stdin,
                 if (ok)
                     pcrl = NULL;
             } else if (pcrls != NULL) {
-                ok = sk_X509_CRL_push(*pcrls, OSSL_STORE_INFO_get1_CRL(info));
+                X509_CRL *crl = OSSL_STORE_INFO_get1_CRL(info);
+
+                ok = crl != NULL && sk_X509_CRL_push(*pcrls, crl);
+                if (!ok)
+                    X509_CRL_free(crl);
             }
             ncrls += ok;
             break;

diff --git a/test/build.info b/test/build.info
index 742737d87368531a54251953b527dcfc3c9372f6..6255b93e4044b57296cd2c5180456d43ca385138 100644 (file)
--- a/test/build.info
+++ b/test/build.info
@@ -82,7 +82,7 @@ IF[{- !$disabled{tests} -}]
   ENDIF
 
   IF[{- !$disabled{'allocfail-tests'} -}]
-    PROGRAMS{noninst}=handshake-memfail x509-memfail
+    PROGRAMS{noinst}=handshake-memfail x509-memfail load_key_certs_crls_memfail
   ENDIF
 
   IF[{- !$disabled{quic} -}]
@@ -621,6 +621,13 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[x509-memfail]=../include ../apps/include
   DEPEND[x509-memfail]=../libcrypto.a libtestutil.a
 
+  SOURCE[load_key_certs_crls_memfail]=load_key_certs_crls_memfail.c ../apps/lib/apps.c \
+          ../apps/lib/app_rand.c ../apps/lib/app_provider.c ../apps/lib/app_libctx.c \
+          ../apps/lib/fmt.c ../apps/lib/apps_ui.c ../apps/lib/app_x509.c \
+          ../crypto/asn1/a_time.c ../crypto/ctype.c
+  INCLUDE[load_key_certs_crls_memfail]=.. ../include ../apps/include
+  DEPEND[load_key_certs_crls_memfail]=libtestutil.a ../libcrypto.a ../libssl.a
+
   SOURCE[ssl_handshake_rtt_test]=ssl_handshake_rtt_test.c helpers/ssltestlib.c
   INCLUDE[ssl_handshake_rtt_test]=../include ../apps/include ..
   DEPEND[ssl_handshake_rtt_test]=../libcrypto.a ../libssl.a libtestutil.a

diff --git a/test/load_key_certs_crls_memfail.c b/test/load_key_certs_crls_memfail.c
new file mode 100644 (file)
index 0000000..94297c0
--- /dev/null
+++ b/test/load_key_certs_crls_memfail.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright 2025 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License"); you may not use
+ * this file except in compliance with the License.  You may obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ *
+ * Regression test for issue #30364: memory leak in load_key_certs_crls()
+ * when X509_add_cert() or sk_X509_CRL_push() fails. Exercises the add/push
+ * path under OPENSSL_MALLOC_FAILURES so that with the fix the cert/CRL is
+ * freed on failure (memory_sanitizer would report a leak without the fix).
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include <openssl/x509.h>
+#include <openssl/crypto.h>
+#include "apps.h"
+#include "app_libctx.h"
+#include "testutil.h"
+
+char *default_config_file = NULL;
+
+static char *certfile = NULL;
+static int mcount, rcount, fcount, scount;
+
+static int do_load_key_certs_crls(int allow_failure)
+{
+    STACK_OF(X509) *certs = NULL;
+    int ret = (allow_failure == 1) ? 0 : 1;
+    char uri[1024];
+
+    if (certfile == NULL)
+        return 0;
+
+    (void)snprintf(uri, sizeof(uri), "file:%s", certfile);
+    if (!TEST_true(load_key_certs_crls(uri, FORMAT_UNDEF, 0, NULL, "cert",
+            1, NULL, NULL, NULL, NULL, &certs,
+            NULL, NULL, NULL)))
+        goto err;
+
+    ret = 1;
+err:
+    sk_X509_pop_free(certs, X509_free);
+    return ret;
+}
+
+static int test_record_alloc_counts(void)
+{
+    return do_load_key_certs_crls(1);
+}
+
+static int test_alloc_failures(void)
+{
+    return do_load_key_certs_crls(0);
+}
+
+static int test_report_alloc_counts(void)
+{
+    CRYPTO_get_alloc_counts(&mcount, &rcount, &fcount);
+    TEST_info("skip: %d count %d\n", scount, mcount - scount);
+    return 1;
+}
+
+int setup_tests(void)
+{
+    int ret = 0;
+    char *opmode = NULL;
+
+    if (app_create_libctx() == NULL)
+        return 0;
+
+    if (!TEST_ptr(opmode = test_get_argument(0)))
+        goto err;
+
+    if (!TEST_ptr(certfile = test_get_argument(1)))
+        goto err;
+
+    if (strcmp(opmode, "count") == 0) {
+        CRYPTO_get_alloc_counts(&scount, &rcount, &fcount);
+        ADD_TEST(test_record_alloc_counts);
+        ADD_TEST(test_report_alloc_counts);
+    } else {
+        ADD_TEST(test_alloc_failures);
+    }
+    ret = 1;
+err:
+    return ret;
+}
+
+void cleanup_tests(void)
+{
+}

diff --git a/test/recipes/90-test_memfail.t b/test/recipes/90-test_memfail.t
index fd168fe45abcf03c0d4ddf1ccfbae603ff9ff1ac..d8bf1e7a4b98d8fb1781feab888cd1ba54c86c24 100644 (file)
--- a/test/recipes/90-test_memfail.t
+++ b/test/recipes/90-test_memfail.t
@@ -30,28 +30,25 @@ run(test(["handshake-memfail", "count", srctop_dir("test", "certs")], stderr =>
 
 run(test(["x509-memfail", "count", srctop_file("test", "certs", "servercert.pem")], stderr => "$resultdir/x509countinfo.txt"));
 
+run(test(["load_key_certs_crls_memfail", "count", srctop_file("test", "certs", "servercert.pem")], stderr => "$resultdir/load_key_certs_crls_countinfo.txt"));
+
 sub get_count_info {
     my ($infile) = @_;
-    my @vals;
+    my ($skipcount, $malloccount) = (0, 0);
 
-    # Read in our input file
-    open my $handle, '<', "$infile";
+    open my $handle, '<', "$infile" or return (0, 0);
     chomp(my @lines = <$handle>);
     close $handle;
 
-    # parse the input file
-    foreach(@lines) {
-        if ($_ =~/skip:/) {
-            @vals = split ' ', $_;
+    # Match the test program output: "skip: <number> count <number>"
+    # Stderr may be captured with a "# " prefix per line (TAP-style).
+    foreach (@lines) {
+        if (/\bskip:\s*(\d+)\s+count\s+(\d+)/) {
+            $skipcount = $1;
+            $malloccount = $2;
             last;
         }
     }
-    #
-    #The number of allocations we skip is in argument 2
-    #The number of mallocs we should test is in argument 4
-    #
-    my $skipcount = $vals[2];
-    my $malloccount = $vals[4];
     return ($skipcount, $malloccount);
 }
 
@@ -59,11 +56,17 @@ my ($hsskipcount, $hsmalloccount) = get_count_info("$resultdir/hscountinfo.txt")
 
 my ($x509skipcount, $x509malloccount) = get_count_info("$resultdir/x509countinfo.txt");
 
+my ($load_key_certs_crls_skipcount, $load_key_certs_crls_malloccount) = get_count_info("$resultdir/load_key_certs_crls_countinfo.txt");
+
+my $total_malloccount = $hsmalloccount + $x509malloccount + $load_key_certs_crls_malloccount;
+plan skip_all => "could not get malloc counts (one or more count runs failed or output format changed)"
+    if $total_malloccount == 0;
+
 #
 # Now we can plan our tests.  We plan to run malloccount iterations of this
 # test
 #
-plan tests => $hsmalloccount + $x509malloccount;
+plan tests => $total_malloccount;
 
 sub run_memfail_test {
     my $skipcount = $_[0];
@@ -88,3 +91,5 @@ run_memfail_test($hsskipcount, $hsmalloccount, ["handshake-memfail", "run", srct
 
 run_memfail_test($x509skipcount, $x509malloccount, ["x509-memfail", "run", srctop_file("test", "certs", "servercert.pem")]);
 
+run_memfail_test($load_key_certs_crls_skipcount, $load_key_certs_crls_malloccount, ["load_key_certs_crls_memfail", "run", srctop_file("test", "certs", "servercert.pem")]);
+