diff options
author | Neil Horman <nhorman@openssl.org> | 2023-12-07 16:56:39 -0500 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2024-01-12 10:39:39 +0100 |
commit | 12726997e86dc8f19c011ab8cbd995c10b12547d (patch) | |
tree | 29976f047c996a3f3a5ab46d250e7d39fb906bd5 | |
parent | 1ce66c7fbad8abcc19aef1fffc07dd453722b98e (diff) |
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)
-rw-r--r-- | crypto/asn1/asn_mstbl.c | 6 | ||||
-rw-r--r-- | test/asn1_stable_parse_test.c | 81 | ||||
-rw-r--r-- | test/build.info | 6 | ||||
-rw-r--r-- | test/recipes/04-test_asn1_stable_parse.t | 24 | ||||
-rw-r--r-- | test/recipes/04-test_asn1_stable_parse_data/asn1_stable_parse.cnf | 16 |
5 files changed, 131 insertions, 2 deletions
diff --git a/crypto/asn1/asn_mstbl.c b/crypto/asn1/asn_mstbl.c index 3543cd2256..cc58c193cd 100644 --- 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 index 0000000000..491e575edd --- /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 6a350ffe9f..416c227077 100644 --- 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 index 0000000000..a6fe6a3d8f --- /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 index 0000000000..28381a083b --- /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 + |