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)

(cherry picked from commit bac7e687d71b124b09ad6ad3e15be9b38c08a1ba)

diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c
index db010720741c7acecca912eb02627246df85f2d0..1340dbe5d2f3d9c417d748a3a3014bbc0eadbfcb 100644 (file)
--- 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 4a719d4d118e2824e6a85d39cad0b275d8c6533c..4d4c8d32515c241108f444762a7734e1e8e567f6 100644 (file)
--- 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 0289df4de78945977025de888b3bb3156c9d1d43..2db7294217b10ff9f2a09413cdb2eaa93fb7b526 100644 (file)
--- 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 7bd49e6b5c2b3f74435bdaafa82f59181454444b..d6139300d3c60abb18bc015fc8cc4c1f2a2a4249 100644 (file)
--- 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 (file)
index 0000000..f982edb
--- /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 95df179bbe76832ab0443124e8fa956a363223eb..b491acb1da65319c9e16077428f4f45306e947b3 100644 (file)
--- 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";