]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
crypto/x509/x509_lu.c: fix memory leak in obj_ht_foreach_object()
authorNikola Pajkovsky <nikolap@openssl.org>
Tue, 30 Jun 2026 06:55:09 +0000 (08:55 +0200)
committerNikola Pajkovsky <nikolap@openssl.org>
Tue, 7 Jul 2026 07:41:59 +0000 (09:41 +0200)
when sk_X509_OBJECT_push() fails after x509_object_dup() has already
allocated the duplicate, the dup is neither stored on the destination
stack nor freed: the error path only pop_free()s the stack the dup was
never pushed onto, so it is leaked.

Set env ASAN_OPTIONS in test explicitly to detect_leaks=1 to force
ASAN to fail the test. Otherwise, the test reports ok even with valid
leak.

Fixes: 08cecb4448e9 "Add X509_STORE_get1_objects"
Fixes: https://github.com/openssl/openssl/issues/31771
Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
Reviewed-by: Norbert Pocs <norbertp@openssl.org>
Reviewed-by: Bob Beck <beck@openssl.org>
MergeDate: Tue Jul  7 07:42:17 2026
(Merged from https://github.com/openssl/openssl/pull/31784)

crypto/x509/x509_lu.c
test/recipes/60-test_x509_load_cert_file.t
test/x509_load_cert_file_test.c

index e9cc7145e36d016bbcb849db1580bb60752907a1..01e1969d196d36e00eea47fc20a77b42bfd38eac 100644 (file)
@@ -764,6 +764,7 @@ static int obj_ht_foreach_object(HT_VALUE *v, void *arg)
     return 1;
 
 err:
+    X509_OBJECT_free(dup);
     sk_X509_OBJECT_pop_free(*sk, X509_OBJECT_free);
     *sk = NULL;
 
index e329d7675c4ee27a1ccb49686ed22ea7987665f4..15e9908ce1b6eeafd0f4b01323ab7b666a606474 100644 (file)
@@ -8,6 +8,8 @@
 
 use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
+$ENV{ASAN_OPTIONS} = "detect_leaks=1";
+
 setup("test_load_cert_file");
 
 plan tests => 1;
index 0df654559aff1b11caaf2401691cfc07e01bee11..f9656aaa31c58be3b7d42c77a8f6134116f7c850 100644 (file)
@@ -198,6 +198,38 @@ err:
     return ret;
 }
 
+static int test_x509_get1_objects_mfail(void)
+{
+    X509 *cert1 = NULL, *cert2 = NULL;
+    X509_STORE *store = NULL;
+    STACK_OF(X509_OBJECT) *objs = NULL;
+    int ret = 0;
+
+    if (!TEST_ptr(cert1 = X509_from_strings(cn_cert1))
+        || !TEST_ptr(cert2 = X509_from_strings(cn_cert2)))
+        goto err;
+
+    store = X509_STORE_new();
+    if (!TEST_ptr(store))
+        goto err;
+    if (!TEST_true(X509_STORE_add_cert(store, cert1))
+        || !TEST_true(X509_STORE_add_cert(store, cert2)))
+        goto err;
+
+    MFAIL_start();
+    objs = X509_STORE_get1_objects(store);
+    MFAIL_end();
+
+    ret = (objs != NULL);
+
+err:
+    sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free);
+    X509_STORE_free(store);
+    X509_free(cert1);
+    X509_free(cert2);
+    return ret;
+}
+
 OPT_TEST_DECLARE_USAGE("cert.pem [crl.pem]\n")
 
 int setup_tests(void)
@@ -216,6 +248,7 @@ int setup_tests(void)
     ADD_TEST(test_load_cert_file);
     ADD_TEST(test_load_same_cn_certs);
     ADD_MFAIL_TEST(test_x509_store_add_mfail);
+    ADD_MFAIL_TEST(test_x509_get1_objects_mfail);
 
     return 1;
 }