From a693d69cd8b4b88a81f3e8174dcf90bbed4cbb53 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 2 Jan 2024 15:48:00 -0500 Subject: Validate config options during x509 extension creation There are several points during x509 extension creation which rely on configuration options which may have been incorrectly parsed due to invalid settings. Preform a value check for null in those locations to avoid various crashes/undefined behaviors Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/23183) (cherry picked from commit bac7e687d71b124b09ad6ad3e15be9b38c08a1ba) --- crypto/x509/v3_addr.c | 4 ++++ crypto/x509/v3_asid.c | 5 +++++ crypto/x509/v3_crld.c | 5 +++++ crypto/x509/v3_ist.c | 16 ++++++++++++---- test/invalid-x509.cnf | 6 ++++++ test/recipes/25-test_x509.t | 10 +++++++++- 6 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 test/invalid-x509.cnf diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c index db01072074..1340dbe5d2 100644 --- a/crypto/x509/v3_addr.c +++ b/crypto/x509/v3_addr.c @@ -972,6 +972,10 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, * the other input values. */ if (safi != NULL) { + if (val->value == NULL) { + ERR_raise(ERR_LIB_X509V3, X509V3_R_MISSING_VALUE); + goto err; + } *safi = strtoul(val->value, &t, 0); t += strspn(t, " \t"); if (*safi > 0xFF || *t++ != ':') { diff --git a/crypto/x509/v3_asid.c b/crypto/x509/v3_asid.c index 4a719d4d11..4d4c8d3251 100644 --- a/crypto/x509/v3_asid.c +++ b/crypto/x509/v3_asid.c @@ -547,6 +547,11 @@ static void *v2i_ASIdentifiers(const struct v3_ext_method *method, goto err; } + if (val->value == NULL) { + ERR_raise(ERR_LIB_X509V3, X509V3_R_EXTENSION_VALUE_ERROR); + goto err; + } + /* * Handle inheritance. */ diff --git a/crypto/x509/v3_crld.c b/crypto/x509/v3_crld.c index 0289df4de7..2db7294217 100644 --- a/crypto/x509/v3_crld.c +++ b/crypto/x509/v3_crld.c @@ -70,6 +70,11 @@ static int set_dist_point_name(DIST_POINT_NAME **pdp, X509V3_CTX *ctx, STACK_OF(GENERAL_NAME) *fnm = NULL; STACK_OF(X509_NAME_ENTRY) *rnm = NULL; + if (cnf->value == NULL) { + ERR_raise(ERR_LIB_X509V3, X509V3_R_MISSING_VALUE); + goto err; + } + if (strncmp(cnf->name, "fullname", 9) == 0) { fnm = gnames_from_sectname(ctx, cnf->value); if (!fnm) diff --git a/crypto/x509/v3_ist.c b/crypto/x509/v3_ist.c index 7bd49e6b5c..d6139300d3 100644 --- a/crypto/x509/v3_ist.c +++ b/crypto/x509/v3_ist.c @@ -50,25 +50,33 @@ static ISSUER_SIGN_TOOL *v2i_issuer_sign_tool(X509V3_EXT_METHOD *method, X509V3_ } if (strcmp(cnf->name, "signTool") == 0) { ist->signTool = ASN1_UTF8STRING_new(); - if (ist->signTool == NULL || !ASN1_STRING_set(ist->signTool, cnf->value, strlen(cnf->value))) { + if (ist->signTool == NULL + || cnf->value == NULL + || !ASN1_STRING_set(ist->signTool, cnf->value, strlen(cnf->value))) { ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE); goto err; } } else if (strcmp(cnf->name, "cATool") == 0) { ist->cATool = ASN1_UTF8STRING_new(); - if (ist->cATool == NULL || !ASN1_STRING_set(ist->cATool, cnf->value, strlen(cnf->value))) { + if (ist->cATool == NULL + || cnf->value == NULL + || !ASN1_STRING_set(ist->cATool, cnf->value, strlen(cnf->value))) { ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE); goto err; } } else if (strcmp(cnf->name, "signToolCert") == 0) { ist->signToolCert = ASN1_UTF8STRING_new(); - if (ist->signToolCert == NULL || !ASN1_STRING_set(ist->signToolCert, cnf->value, strlen(cnf->value))) { + if (ist->signToolCert == NULL + || cnf->value == NULL + || !ASN1_STRING_set(ist->signToolCert, cnf->value, strlen(cnf->value))) { ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE); goto err; } } else if (strcmp(cnf->name, "cAToolCert") == 0) { ist->cAToolCert = ASN1_UTF8STRING_new(); - if (ist->cAToolCert == NULL || !ASN1_STRING_set(ist->cAToolCert, cnf->value, strlen(cnf->value))) { + if (ist->cAToolCert == NULL + || cnf->value == NULL + || !ASN1_STRING_set(ist->cAToolCert, cnf->value, strlen(cnf->value))) { ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE); goto err; } diff --git a/test/invalid-x509.cnf b/test/invalid-x509.cnf new file mode 100644 index 0000000000..f982edb979 --- /dev/null +++ b/test/invalid-x509.cnf @@ -0,0 +1,6 @@ +[ext] +issuerSignTool = signTool +sbgp-autonomousSysNum = AS +issuingDistributionPoint = fullname +sbgp-ipAddrBlock = IPv4-SAFI + diff --git a/test/recipes/25-test_x509.t b/test/recipes/25-test_x509.t index 95df179bbe..b491acb1da 100644 --- a/test/recipes/25-test_x509.t +++ b/test/recipes/25-test_x509.t @@ -16,7 +16,7 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/; setup("test_x509"); -plan tests => 28; +plan tests => 29; # Prevent MSys2 filename munging for arguments that look like file paths but # aren't @@ -186,6 +186,14 @@ ok(run(app(["openssl", "x509", "-in", $a_cert, "-CA", $ca_cert, # verify issuer is CA ok (get_issuer($a2_cert) =~ /CN = ca.example.com/); +my $in_csr = srctop_file('test', 'certs', 'x509-check.csr'); +my $in_key = srctop_file('test', 'certs', 'x509-check-key.pem'); +my $invextfile = srctop_file('test', 'invalid-x509.cnf'); +# Test that invalid extensions settings fail +ok(!run(app(["openssl", "x509", "-req", "-in", $in_csr, "-signkey", $in_key, + "-out", "/dev/null", "-days", "3650" , "-extensions", "ext", + "-extfile", $invextfile]))); + # Tests for issue #16080 (fixed in 1.1.1o) my $b_key = "b-key.pem"; my $b_csr = "b-cert.csr"; -- cgit v1.2.3