Fix NULL pointer deref when parsing the stable section

When parsing the stable section of a config such as this:
openssl_conf = openssl_init
[openssl_init]
stbl_section = mstbl
[mstbl]
id-tc26 = min

Can lead to a SIGSEGV, as the parsing code doesnt recognize min as a
proper section name without a trailing colon to associate it with a
value.  As a result the stack of configuration values has an entry with
a null value in it, which leads to the SIGSEGV in do_tcreate when we
attempt to pass NULL to strtoul.

Fix it by skipping any entry in the config name/value list that has a
null value, prior to passing it to stroul

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22988)

(cherry picked from commit 0981c20f8efa68bf9d68d7715280f83812c19a7e)

diff --git a/crypto/asn1/asn_mstbl.c b/crypto/asn1/asn_mstbl.c
index 3543cd22568fd0ea5b0a603854edf6487fc64100..cc58c193cd7802dafd08d80d9fd4f667a74bea5b 100644 (file)
--- a/crypto/asn1/asn_mstbl.c
+++ b/crypto/asn1/asn_mstbl.c
@@ -72,6 +72,8 @@ static int do_tcreate(const char *value, const char *name)
         goto err;
     for (i = 0; i < sk_CONF_VALUE_num(lst); i++) {
         cnf = sk_CONF_VALUE_value(lst, i);
+        if (cnf->value == NULL)
+            goto err;
         if (strcmp(cnf->name, "min") == 0) {
             tbl_min = strtoul(cnf->value, &eptr, 0);
             if (*eptr)
@@ -98,7 +100,9 @@ static int do_tcreate(const char *value, const char *name)
     if (rv == 0) {
         if (cnf)
             ERR_raise_data(ERR_LIB_ASN1, ASN1_R_INVALID_STRING_TABLE_VALUE,
-                           "field=%s, value=%s", cnf->name, cnf->value);
+                           "field=%s, value=%s", cnf->name,
+                                                 cnf->value != NULL ? cnf->value
+                                                 : value);
         else
             ERR_raise_data(ERR_LIB_ASN1, ASN1_R_INVALID_STRING_TABLE_VALUE,
                            "name=%s, value=%s", name, value);

diff --git a/test/asn1_stable_parse_test.c b/test/asn1_stable_parse_test.c
new file mode 100644 (file)
index 0000000..491e575
--- /dev/null
+++ b/test/asn1_stable_parse_test.c
@@ -0,0 +1,81 @@
+/*
+ * Copyright 2023 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 can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <openssl/evp.h>
+#include "testutil.h"
+
+static char *config_file = NULL;
+
+typedef enum OPTION_choice {
+    OPT_ERR = -1,
+    OPT_EOF = 0,
+    OPT_CONFIG_FILE,
+    OPT_TEST_ENUM
+} OPTION_CHOICE;
+
+const OPTIONS *test_get_options(void)
+{
+    static const OPTIONS options[] = {
+        OPT_TEST_OPTIONS_DEFAULT_USAGE,
+        { "config", OPT_CONFIG_FILE, '<',
+          "The configuration file to use for the libctx" },
+        { NULL }
+    };
+    return options;
+}
+
+
+/*
+ * Test that parsing a config file with incorrect stable settings aren't parsed
+ * and appropriate errors are raised
+ */
+static int test_asn1_stable_parse(void)
+{
+    int testret = 0;
+    unsigned long errcode;
+    OSSL_LIB_CTX *newctx = OSSL_LIB_CTX_new();
+
+    if (!TEST_ptr(newctx))
+        goto out;
+
+    if (!TEST_int_eq(OSSL_LIB_CTX_load_config(newctx, config_file), 0))
+        goto err;
+
+    errcode = ERR_peek_error();
+    if (ERR_GET_LIB(errcode) != ERR_LIB_ASN1)
+        goto err;
+    if (ERR_GET_REASON(errcode) != ASN1_R_INVALID_STRING_TABLE_VALUE)
+        goto err;
+
+    ERR_clear_error();
+
+    testret = 1;
+err:
+    OSSL_LIB_CTX_free(newctx);
+out:
+    return testret;
+}
+
+int setup_tests(void)
+{
+    OPTION_CHOICE o;
+
+    while ((o = opt_next()) != OPT_EOF) {
+        switch (o) {
+        case OPT_CONFIG_FILE:
+            config_file = opt_arg();
+            break;
+        default:
+            return 0;
+        }
+    }
+
+    ADD_TEST(test_asn1_stable_parse);
+    return 1;
+}

diff --git a/test/build.info b/test/build.info
index 82740dcb6e4aa9430d990cc11de62b4e9a18e380..81fd75df15c61c12e81f5846370f915a8f9f68e8 100644 (file)
--- a/test/build.info
+++ b/test/build.info
@@ -51,7 +51,7 @@ IF[{- !$disabled{tests} -}]
           bioprinttest sslapitest dtlstest sslcorrupttest \
           bio_enc_test pkey_meth_test pkey_meth_kdf_test evp_kdf_test uitest \
           cipherbytes_test threadstest_fips \
-          asn1_encode_test asn1_decode_test asn1_string_table_test \
+          asn1_encode_test asn1_decode_test asn1_string_table_test asn1_stable_parse_test \
           x509_time_test x509_dup_cert_test x509_check_cert_pkey_test \
           recordlentest drbgtest rand_status_test sslbuffertest \
           time_offset_test pemtest ssl_cert_table_internal_test ciphername_test \
@@ -545,6 +545,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[asn1_string_table_test]=../include ../apps/include
   DEPEND[asn1_string_table_test]=../libcrypto libtestutil.a
 
+  SOURCE[asn1_stable_parse_test]=asn1_stable_parse_test.c
+  INCLUDE[asn1_stable_parse_test]=../include ../apps/include
+  DEPEND[asn1_stable_parse_test]=../libcrypto libtestutil.a
+
   SOURCE[time_offset_test]=time_offset_test.c
   INCLUDE[time_offset_test]=../include ../apps/include
   DEPEND[time_offset_test]=../libcrypto libtestutil.a

diff --git a/test/recipes/04-test_asn1_stable_parse.t b/test/recipes/04-test_asn1_stable_parse.t
new file mode 100644 (file)
index 0000000..a6fe6a3
--- /dev/null
+++ b/test/recipes/04-test_asn1_stable_parse.t
@@ -0,0 +1,24 @@
+#! /usr/bin/env perl
+# Copyright 2023 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 can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+
+use OpenSSL::Test::Simple;
+use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir bldtop_dir bldtop_file data_dir/;
+use OpenSSL::Test::Utils;
+use Cwd qw(abs_path);
+
+BEGIN {
+setup("test_asn1_stable_parse");
+}
+my $config_path = srctop_file("test", "recipes", "04-test_asn1_stable_parse_data", "asn1_stable_parse.cnf");
+
+plan tests => 1;
+
+ok(run(test(["asn1_stable_parse_test", "-config", $config_path])),
+   "Confirm that malformed entries in stable section are not parsed");
+

diff --git a/test/recipes/04-test_asn1_stable_parse_data/asn1_stable_parse.cnf b/test/recipes/04-test_asn1_stable_parse_data/asn1_stable_parse.cnf
new file mode 100644 (file)
index 0000000..28381a0
--- /dev/null
+++ b/test/recipes/04-test_asn1_stable_parse_data/asn1_stable_parse.cnf
@@ -0,0 +1,16 @@
+openssl_conf = openssl_init
+config_diagnostics = 1
+
+[openssl_init]
+s = mstbl
+
+[mstbl]
+id-tc26 = min
+id-tc27 = ::::::
+id-tc28 = ,,,,,,
+id-tc29 = :,:,:,
+id-tc30 = n1:min
+id-tc31 = n2:max
+id-tc32 = n3:
+id-tc33 = :0
+