Commit 7174737196 for openssl.org
commit 7174737196dfc563c14719f51f7187dfda90dbe3
Author: Viktor Dukhovni <viktor@openssl.org>
Date: Thu Nov 12 01:26:06 2026 +1100
Fix PEM_X509_INFO_read_bio_ex() failure with a caller-supplied stack
When the caller passes in an existing stack and reading fails part
way, the error path freed every entry in that stack, including
the ones the caller already had, but left them in the stack.
Freeing the stack afterwards then freed those entries a second
time.
On failure, remove and free only the entries added by this call.
The caller's original entries are left untouched.
Add tests for a failing append, a failing append after some entries
were added, and a successful append.
Fixes: CVE-2026-107493
Co-authored-by: Pocs Norbert <norbertpocs0@gmail.com>
Assisted-by: Claude Code:claude-sonnet-5-5
Reviewed-by: Norbert Pocs <norbertp@openssl.org>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Andrew Dinh <andrewd@openssl.org>
Reviewed-by: Bob Beck <beck@openssl.org>
Merge-date: Sat Nov 14 04:08:43 2026
Merged-from: https://github.com/openssl/openssl/pull/33662
diff --git a/crypto/pem/pem_info.c b/crypto/pem/pem_info.c
index 281dfd69ed..67c963c8ee 100644
--- a/crypto/pem/pem_info.c
+++ b/crypto/pem/pem_info.c
@@ -68,7 +68,7 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
unsigned char *data = NULL;
const unsigned char *p;
long len, error = 0;
- int ok = 0;
+ int ok = 0, preserve_cnt = 0;
STACK_OF(X509_INFO) *ret = NULL;
unsigned int i, raw, ptype;
pem_info_type itype = PEM_INFO_NONE;
@@ -78,8 +78,10 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
ERR_raise(ERR_LIB_PEM, ERR_R_CRYPTO_LIB);
goto err;
}
- } else
+ } else {
ret = sk;
+ preserve_cnt = sk_X509_INFO_num(sk);
+ }
if ((xi = X509_INFO_new()) == NULL)
goto err;
@@ -229,10 +231,8 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
err:
X509_INFO_free(xi);
if (!ok) {
- for (i = 0; ((int)i) < sk_X509_INFO_num(ret); i++) {
- xi = sk_X509_INFO_value(ret, i);
- X509_INFO_free(xi);
- }
+ while (sk_X509_INFO_num(ret) > preserve_cnt)
+ X509_INFO_free(sk_X509_INFO_pop(ret));
if (ret != sk)
sk_X509_INFO_free(ret);
ret = NULL;
diff --git a/test/x509_check_cert_pkey_test.c b/test/x509_check_cert_pkey_test.c
index 4c6c0c4fb5..a84dbf8a7b 100644
--- a/test/x509_check_cert_pkey_test.c
+++ b/test/x509_check_cert_pkey_test.c
@@ -133,6 +133,128 @@ static int test_PEM_X509_INFO_read_bio(void)
return TEST_int_eq(count, expected);
}
+/*
+ * Counts the X509 objects that were tagged via X509_set_ex_data() and have
+ * since been freed, i.e. whose last reference was dropped.
+ */
+static int x509_ex_idx = -1;
+static int tagged_freed;
+
+static void tagged_free_cb(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
+ int idx, long argl, void *argp)
+{
+ if (ptr != NULL)
+ (*(int *)argp)++;
+}
+
+/*
+ * Build the PEM input for one append test:
+ *
+ * 0: a single malformed certificate (valid PEM armour, invalid DER)
+ * 1: the contents of |file|, then a malformed certificate
+ * 2: the contents of |file| only
+ */
+static BIO *append_input(int idx)
+{
+ static const char malformed[] = "-----BEGIN CERTIFICATE-----\n"
+ "MAA=\n"
+ "-----END CERTIFICATE-----\n";
+ BIO *mem = NULL, *in = NULL;
+ char buf[4096];
+ int n;
+
+ if (!TEST_ptr(mem = BIO_new(BIO_s_mem())))
+ return NULL;
+ if (idx != 0) {
+ if (!TEST_ptr(in = BIO_new_file(file, "r")))
+ goto err;
+ while ((n = BIO_read(in, buf, sizeof(buf))) > 0)
+ if (!TEST_int_eq(BIO_write(mem, buf, n), n))
+ goto err;
+ BIO_free(in);
+ in = NULL;
+ }
+ if (idx != 2 && !TEST_int_gt(BIO_puts(mem, malformed), 0))
+ goto err;
+ return mem;
+err:
+ BIO_free(in);
+ BIO_free(mem);
+ return NULL;
+}
+
+/*
+ * Appending to a caller-owned stack must leave the entries that were already
+ * in the stack alone when the append fails, and must roll back any entries
+ * added by the failed call.
+ *
+ * To check that the pre-existing entries are not freed, the test holds an
+ * extra reference on the first certificate in the stack. After the call the
+ * stack must still own its reference, so dropping ours must not free the
+ * certificate, and freeing the stack afterwards must.
+ */
+static int test_PEM_X509_INFO_read_bio_append(int idx)
+{
+ BIO *in = NULL, *mem = NULL;
+ STACK_OF(X509_INFO) *sk = NULL, *ret;
+ X509 *held = NULL;
+ int i, n0, tagged = 0, ok = 0;
+ int expect_ok = (idx == 2);
+
+ tagged_freed = 0;
+ if (!TEST_ptr(in = BIO_new_file(file, "r"))
+ || !TEST_ptr(sk = PEM_X509_INFO_read_bio(in, NULL, NULL, "")))
+ goto err;
+ n0 = sk_X509_INFO_num(sk);
+
+ /* Hold a tagged reference on the first certificate, if there is one. */
+ for (i = 0; i < n0; i++) {
+ X509_INFO *it = sk_X509_INFO_value(sk, i);
+
+ if (it->x509 != NULL) {
+ held = it->x509;
+ if (!TEST_true(X509_up_ref(held))
+ || !TEST_true(X509_set_ex_data(held, x509_ex_idx, held)))
+ goto err;
+ tagged = 1;
+ break;
+ }
+ }
+
+ if (!TEST_ptr(mem = append_input(idx)))
+ goto err;
+ ret = PEM_X509_INFO_read_bio(mem, sk, NULL, "");
+
+ if (expect_ok) {
+ if (!TEST_ptr_eq(ret, sk)
+ || !TEST_int_eq(sk_X509_INFO_num(sk), 2 * n0))
+ goto err;
+ } else {
+ if (!TEST_ptr_null(ret)
+ || !TEST_int_eq(sk_X509_INFO_num(sk), n0))
+ goto err;
+ }
+
+ if (tagged) {
+ /* Our reference must not have been the stack's last one. */
+ X509_free(held);
+ held = NULL;
+ if (!TEST_int_eq(tagged_freed, 0))
+ goto err;
+ }
+ sk_X509_INFO_pop_free(sk, X509_INFO_free);
+ sk = NULL;
+ if (tagged && !TEST_int_eq(tagged_freed, 1))
+ goto err;
+ ok = 1;
+err:
+ X509_free(held);
+ sk_X509_INFO_pop_free(sk, X509_INFO_free);
+ BIO_free(in);
+ BIO_free(mem);
+ return ok;
+}
+
const OPTIONS *test_get_options(void)
{
enum { OPT_TEST_ENUM };
@@ -165,7 +287,12 @@ int setup_tests(void)
return 0;
if (!TEST_int_eq(sscanf(num, "%d", &expected), 1))
return 0;
+ x509_ex_idx = X509_get_ex_new_index(0, &tagged_freed, NULL, NULL,
+ tagged_free_cb);
+ if (!TEST_int_ge(x509_ex_idx, 0))
+ return 0;
ADD_TEST(test_PEM_X509_INFO_read_bio);
+ ADD_ALL_TESTS(test_PEM_X509_INFO_read_bio_append, 3);
return 1;
}