Skip to content

Commit

Permalink
Fix NULL pointer deref when parsing the stable section
Browse files Browse the repository at this point in the history
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 #22988)
  • Loading branch information
nhorman authored and t8m committed Jan 12, 2024
1 parent 3cb1b51 commit 0981c20
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 2 deletions.
6 changes: 5 additions & 1 deletion crypto/asn1/asn_mstbl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand Down
81 changes: 81 additions & 0 deletions test/asn1_stable_parse_test.c
Original file line number Diff line number Diff line change
@@ -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;
}
6 changes: 5 additions & 1 deletion test/build.info
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions test/recipes/04-test_asn1_stable_parse.t
Original file line number Diff line number Diff line change
@@ -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");

16 changes: 16 additions & 0 deletions test/recipes/04-test_asn1_stable_parse_data/asn1_stable_parse.cnf
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 0981c20

Please sign in to comment.