Fix a crash in X509v3_asid_subset()

If the asnum or rdi fields are NULL and the ASIdentifiers are otherwise
subsets then this will result in a crash. Of note is that rdi will usually
be NULL.

Reported by Theo Buehler (@botovq)

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Yang <kaishen.yy@antfin.com>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from https://github.com/openssl/openssl/pull/18514)

(cherry picked from commit 01fc9b6bce82f0534d6673659a0e59a71f57ee82)

diff --git a/crypto/x509v3/v3_asid.c b/crypto/x509v3/v3_asid.c
index ac6857267291fbe2f61e972fdd5a6c5c89c6ff3b..9bdc682978f0ab90b23c315fea08898a390c1a27 100644 (file)
--- a/crypto/x509v3/v3_asid.c
+++ b/crypto/x509v3/v3_asid.c
@@ -700,15 +700,28 @@ static int asid_contains(ASIdOrRanges *parent, ASIdOrRanges *child)
  */
 int X509v3_asid_subset(ASIdentifiers *a, ASIdentifiers *b)
 {
-    return (a == NULL ||
-            a == b ||
-            (b != NULL &&
-             !X509v3_asid_inherits(a) &&
-             !X509v3_asid_inherits(b) &&
-             asid_contains(b->asnum->u.asIdsOrRanges,
-                           a->asnum->u.asIdsOrRanges) &&
-             asid_contains(b->rdi->u.asIdsOrRanges,
-                           a->rdi->u.asIdsOrRanges)));
+    int subset;
+
+    if (a == NULL || a == b)
+        return 1;
+
+    if (b == NULL)
+        return 0;
+
+    if (X509v3_asid_inherits(a) || X509v3_asid_inherits(b))
+        return 0;
+
+    subset = a->asnum == NULL
+             || (b->asnum != NULL
+                 && asid_contains(b->asnum->u.asIdsOrRanges,
+                                  a->asnum->u.asIdsOrRanges));
+    if (!subset)
+        return 0;
+
+    return a->rdi == NULL
+           || (b->rdi != NULL
+               && asid_contains(b->rdi->u.asIdsOrRanges,
+                                a->rdi->u.asIdsOrRanges));
 }
 
 /*

diff --git a/test/v3ext.c b/test/v3ext.c
index 14ae49969d07bc0442bda18d438897885cda0d2c..1575e923da46ba98aa1f0d985800d4543d6b1464 100644 (file)
--- a/test/v3ext.c
+++ b/test/v3ext.c
@@ -37,11 +37,89 @@ end:
     return ret;
 }
 
+static int test_asid(void)
+{
+    ASN1_INTEGER *val1 = NULL, *val2 = NULL;
+    ASIdentifiers *asid1 = ASIdentifiers_new(), *asid2 = ASIdentifiers_new(),
+                  *asid3 = ASIdentifiers_new(), *asid4 = ASIdentifiers_new();
+    int testresult = 0;
+
+    if (!TEST_ptr(asid1)
+            || !TEST_ptr(asid2)
+            || !TEST_ptr(asid3))
+        goto err;
+
+    if (!TEST_ptr(val1 = ASN1_INTEGER_new())
+            || !TEST_true(ASN1_INTEGER_set_int64(val1, 64496)))
+        goto err;
+
+    if (!TEST_true(X509v3_asid_add_id_or_range(asid1, V3_ASID_ASNUM, val1, NULL)))
+        goto err;
+
+    val1 = NULL;
+    if (!TEST_ptr(val2 = ASN1_INTEGER_new())
+            || !TEST_true(ASN1_INTEGER_set_int64(val2, 64497)))
+        goto err;
+
+    if (!TEST_true(X509v3_asid_add_id_or_range(asid2, V3_ASID_ASNUM, val2, NULL)))
+        goto err;
+
+    val2 = NULL;
+    if (!TEST_ptr(val1 = ASN1_INTEGER_new())
+            || !TEST_true(ASN1_INTEGER_set_int64(val1, 64496))
+            || !TEST_ptr(val2 = ASN1_INTEGER_new())
+            || !TEST_true(ASN1_INTEGER_set_int64(val2, 64497)))
+        goto err;
+
+    /*
+     * Just tests V3_ASID_ASNUM for now. Could be extended at some point to also
+     * test V3_ASID_RDI if we think it is worth it.
+     */
+    if (!TEST_true(X509v3_asid_add_id_or_range(asid3, V3_ASID_ASNUM, val1, val2)))
+        goto err;
+    val1 = val2 = NULL;
+
+    /* Actual subsets */
+    if (!TEST_true(X509v3_asid_subset(NULL, NULL))
+            || !TEST_true(X509v3_asid_subset(NULL, asid1))
+            || !TEST_true(X509v3_asid_subset(asid1, asid1))
+            || !TEST_true(X509v3_asid_subset(asid2, asid2))
+            || !TEST_true(X509v3_asid_subset(asid1, asid3))
+            || !TEST_true(X509v3_asid_subset(asid2, asid3))
+            || !TEST_true(X509v3_asid_subset(asid3, asid3))
+            || !TEST_true(X509v3_asid_subset(asid4, asid1))
+            || !TEST_true(X509v3_asid_subset(asid4, asid2))
+            || !TEST_true(X509v3_asid_subset(asid4, asid3)))
+        goto err;
+
+    /* Not subsets */
+    if (!TEST_false(X509v3_asid_subset(asid1, NULL))
+            || !TEST_false(X509v3_asid_subset(asid1, asid2))
+            || !TEST_false(X509v3_asid_subset(asid2, asid1))
+            || !TEST_false(X509v3_asid_subset(asid3, asid1))
+            || !TEST_false(X509v3_asid_subset(asid3, asid2))
+            || !TEST_false(X509v3_asid_subset(asid1, asid4))
+            || !TEST_false(X509v3_asid_subset(asid2, asid4))
+            || !TEST_false(X509v3_asid_subset(asid3, asid4)))
+        goto err;
+
+    testresult = 1;
+ err:
+    ASN1_INTEGER_free(val1);
+    ASN1_INTEGER_free(val2);
+    ASIdentifiers_free(asid1);
+    ASIdentifiers_free(asid2);
+    ASIdentifiers_free(asid3);
+    ASIdentifiers_free(asid4);
+    return testresult;
+}
+
 int setup_tests(void)
 {
     if (!TEST_ptr(infile = test_get_argument(0)))
         return 0;
 
     ADD_TEST(test_pathlen);
+    ADD_TEST(test_asid);
     return 1;
 }