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 openssl#18514)
ituglib · Jun 15, 2022 · 01fc9b6 · 01fc9b6
1 parent 0feb138
commit 01fc9b6
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 9 deletions.
diff --git a/crypto/x509/v3_asid.c b/crypto/x509/v3_asid.c
@@ -689,15 +689,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
@@ -37,6 +37,83 @@ static int test_pathlen(void)
     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;
+}
+
 OPT_TEST_DECLARE_USAGE("cert.pem\n")
 
 int setup_tests(void)
@@ -50,5 +127,6 @@ int setup_tests(void)
         return 0;
 
     ADD_TEST(test_pathlen);
+    ADD_TEST(test_asid);
     return 1;
 }