From 0981c20f8efa68bf9d68d7715280f83812c19a7e Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Thu, 7 Dec 2023 16:56:39 -0500 Subject: [PATCH] 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22988) --- crypto/asn1/asn_mstbl.c | 6 +- test/asn1_stable_parse_test.c | 81 +++++++++++++++++++ test/build.info | 6 +- test/recipes/04-test_asn1_stable_parse.t | 24 ++++++ .../asn1_stable_parse.cnf | 16 ++++ 5 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 test/asn1_stable_parse_test.c create mode 100644 test/recipes/04-test_asn1_stable_parse.t create mode 100644 test/recipes/04-test_asn1_stable_parse_data/asn1_stable_parse.cnf diff --git a/crypto/asn1/asn_mstbl.c b/crypto/asn1/asn_mstbl.c index 515d8181b6..93c6f61bd9 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 +#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 7e1ce28522..cbd610bc93 100644 --- a/test/build.info +++ b/test/build.info @@ -51,7 +51,7 @@ IF[{- !$disabled{tests} -}] bioprinttest sslapitest ssl_handshake_rtt_test dtlstest sslcorrupttest \ bio_enc_test pkey_meth_test pkey_meth_kdf_test evp_kdf_test uitest \ cipherbytes_test threadstest_fips threadpool_test \ - 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 \ @@ -686,6 +686,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 + -- 2.34.1