From: Neil Horman <nhorman@openssl.org>
Date: Tue, 2 Jan 2024 20:48:00 +0000 (-0500)
Subject: Validate config options during x509 extension creation
X-Git-Tag: openssl-3.3.0-alpha1~349
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bac7e687d71b124b09ad6ad3e15be9b38c08a1ba;p=thirdparty%2Fopenssl.git

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 <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23183)
---

diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c
index da9604cf963..bd937388f36 100644
--- a/crypto/x509/v3_addr.c
+++ b/crypto/x509/v3_addr.c
@@ -988,6 +988,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 251243b7237..1cb892df670 100644
--- a/crypto/x509/v3_asid.c
+++ b/crypto/x509/v3_asid.c
@@ -545,6 +545,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 08df3faf86e..e9f6e08e27a 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 (HAS_PREFIX(cnf->name, "fullname")) {
         fnm = gnames_from_sectname(ctx, cnf->value);
         if (!fnm)
diff --git a/crypto/x509/v3_ist.c b/crypto/x509/v3_ist.c
index 978a0f3ed86..4d5fe82f329 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_ASN1_LIB);
                 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_ASN1_LIB);
                 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_ASN1_LIB);
                 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_ASN1_LIB);
                 goto err;
             }
diff --git a/test/invalid-x509.cnf b/test/invalid-x509.cnf
new file mode 100644
index 00000000000..f982edb9797
--- /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 9bf011c1885..9b11169a982 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 => 43;
+plan tests => 44;
 
 # Prevent MSys2 filename munging for arguments that look like file paths but
 # aren't
@@ -217,6 +217,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";