Fix a crash in v2i_IPAddrBlocks()

If an IP address prefix value is supplied that is too large then a crash
can result. v2i_IPAddrBlocks() should sanity check the prefix value, as
should X509v3_addr_add_prefix().

Reported by Theo Buehler (@botovq)

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18847)

diff --git a/crypto/x509v3/v3_addr.c b/crypto/x509v3/v3_addr.c
index ccce34ef2e48a4fb0e648f87049d9fe57acec5b1..f9c368bea40580c0e4e4ec8f8c2bce794fd1a043 100644 (file)
--- a/crypto/x509v3/v3_addr.c
+++ b/crypto/x509v3/v3_addr.c
@@ -392,12 +392,14 @@ static int range_should_be_prefix(const unsigned char *min,
 /*
  * Construct a prefix.
  */
-static int make_addressPrefix(IPAddressOrRange **result,
-                              unsigned char *addr, const int prefixlen)
+static int make_addressPrefix(IPAddressOrRange **result, unsigned char *addr,
+                              const int prefixlen, const int afilen)
 {
     int bytelen = (prefixlen + 7) / 8, bitlen = prefixlen % 8;
     IPAddressOrRange *aor = IPAddressOrRange_new();
 
+    if (prefixlen < 0 || prefixlen > (afilen * 8))
+        return 0;
     if (aor == NULL)
         return 0;
     aor->type = IPAddressOrRange_addressPrefix;
@@ -437,7 +439,7 @@ static int make_addressRange(IPAddressOrRange **result,
         return 0;
 
     if ((prefixlen = range_should_be_prefix(min, max, length)) >= 0)
-        return make_addressPrefix(result, min, prefixlen);
+        return make_addressPrefix(result, min, prefixlen, length);
 
     if ((aor = IPAddressOrRange_new()) == NULL)
         return 0;
@@ -599,7 +601,9 @@ int X509v3_addr_add_prefix(IPAddrBlocks *addr,
 {
     IPAddressOrRanges *aors = make_prefix_or_range(addr, afi, safi);
     IPAddressOrRange *aor;
-    if (aors == NULL || !make_addressPrefix(&aor, a, prefixlen))
+
+    if (aors == NULL
+            || !make_addressPrefix(&aor, a, prefixlen, length_from_afi(afi)))
         return 0;
     if (sk_IPAddressOrRange_push(aors, aor))
         return 1;
@@ -996,7 +1000,10 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method,
         switch (delim) {
         case '/':
             prefixlen = (int)strtoul(s + i2, &t, 10);
-            if (t == s + i2 || *t != '\0') {
+            if (t == s + i2
+                    || *t != '\0'
+                    || prefixlen > (length * 8)
+                    || prefixlen < 0) {
                 X509V3err(X509V3_F_V2I_IPADDRBLOCKS,
                           X509V3_R_EXTENSION_VALUE_ERROR);
                 X509V3_conf_err(val);

diff --git a/test/v3ext.c b/test/v3ext.c
index 7a240cd706034155eddc4f01f07a2a09237a1939..2bc0f866e2b1036eb16587e980998894f7207208 100644 (file)
--- a/test/v3ext.c
+++ b/test/v3ext.c
@@ -225,6 +225,104 @@ static int test_addr_ranges(void)
     ASN1_OCTET_STRING_free(ip2);
     return testresult;
 }
+
+static struct extvalues_st {
+    const char *value;
+    int pass;
+} extvalues[] = {
+    /* No prefix is ok */
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.1\n", 1 },
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.0/0\n", 1 },
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.0/1\n", 1 },
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.0/32\n", 1 },
+    /* Prefix is too long */
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.0/33\n", 0 },
+    /* Unreasonably large prefix */
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.0/12341234\n", 0 },
+    /* Invalid IP addresses */
+    { "sbgp-ipAddrBlock = IPv4:192.0.0\n", 0 },
+    { "sbgp-ipAddrBlock = IPv4:256.0.0.0\n", 0 },
+    { "sbgp-ipAddrBlock = IPv4:-1.0.0.0\n", 0 },
+    { "sbgp-ipAddrBlock = IPv4:192.0.0.0.0\n", 0 },
+    { "sbgp-ipAddrBlock = IPv3:192.0.0.0\n", 0 },
+
+    /* IPv6 */
+    /* No prefix is ok */
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001::db8\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001:0db8:0000:0000:0000:0000:0000:0000\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::/0\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::/1\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::/32\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001:0db8:0000:0000:0000:0000:0000:0000/32\n", 1 },
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::/128\n", 1 },
+    /* Prefix is too long */
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::/129\n", 0 },
+    /* Unreasonably large prefix */
+    { "sbgp-ipAddrBlock = IPv6:2001:db8::/12341234\n", 0 },
+    /* Invalid IP addresses */
+    /* Not enough blocks of numbers */
+    { "sbgp-ipAddrBlock = IPv6:2001:0db8:0000:0000:0000:0000:0000\n", 0 },
+    /* Too many blocks of numbers */
+    { "sbgp-ipAddrBlock = IPv6:2001:0db8:0000:0000:0000:0000:0000:0000:0000\n", 0 },
+    /* First value too large */
+    { "sbgp-ipAddrBlock = IPv6:1ffff:0db8:0000:0000:0000:0000:0000:0000\n", 0 },
+    /* First value with invalid characters */
+    { "sbgp-ipAddrBlock = IPv6:fffg:0db8:0000:0000:0000:0000:0000:0000\n", 0 },
+    /* First value is negative */
+    { "sbgp-ipAddrBlock = IPv6:-1:0db8:0000:0000:0000:0000:0000:0000\n", 0 }
+};
+
+static int test_ext_syntax(void)
+{
+    size_t i;
+    int testresult = 1;
+
+    for (i = 0; i < OSSL_NELEM(extvalues); i++) {
+        X509V3_CTX ctx;
+        BIO *extbio = BIO_new_mem_buf(extvalues[i].value,
+                                      strlen(extvalues[i].value));
+        CONF *conf;
+        long eline;
+
+        if (!TEST_ptr(extbio))
+            return 0 ;
+
+        conf = NCONF_new(NULL);
+        if (!TEST_ptr(conf)) {
+            BIO_free(extbio);
+            return 0;
+        }
+        if (!TEST_long_gt(NCONF_load_bio(conf, extbio, &eline), 0)) {
+            testresult = 0;
+        } else {
+            X509V3_set_ctx_test(&ctx);
+            X509V3_set_nconf(&ctx, conf);
+
+            if (extvalues[i].pass) {
+                if (!TEST_true(X509V3_EXT_add_nconf(conf, &ctx, "default",
+                                                    NULL))) {
+                    TEST_info("Value: %s", extvalues[i].value);
+                    testresult = 0;
+                }
+            } else {
+                ERR_set_mark();
+                if (!TEST_false(X509V3_EXT_add_nconf(conf, &ctx, "default",
+                                                     NULL))) {
+                    testresult = 0;
+                    TEST_info("Value: %s", extvalues[i].value);
+                    ERR_clear_last_mark();
+                } else {
+                    ERR_pop_to_mark();
+                }
+            }
+        }
+        BIO_free(extbio);
+        NCONF_free(conf);
+    }
+
+    return testresult;
+}
 #endif /* OPENSSL_NO_RFC3779 */
 
 int setup_tests(void)
@@ -236,6 +334,7 @@ int setup_tests(void)
 #ifndef OPENSSL_NO_RFC3779
     ADD_TEST(test_asid);
     ADD_TEST(test_addr_ranges);
+    ADD_TEST(test_ext_syntax);
 #endif /* OPENSSL_NO_RFC3779 */
     return 1;
 }