From b91ad3c69c27c35be4fd7f1e8811c33c31b02afd Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 10 Jun 2022 12:33:45 +0100 Subject: [PATCH] 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 Reviewed-by: Dmitry Belyavskiy (Merged from https://github.com/openssl/openssl/pull/18523) --- crypto/x509/v3_addr.c | 16 ++++--- test/v3ext.c | 99 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c index 51de887a406f9..1697bf7895a0a 100644 --- a/crypto/x509/v3_addr.c +++ b/crypto/x509/v3_addr.c @@ -398,12 +398,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; @@ -440,7 +442,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; @@ -604,7 +606,8 @@ 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; @@ -1009,7 +1012,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) { ERR_raise(ERR_LIB_X509V3, X509V3_R_EXTENSION_VALUE_ERROR); X509V3_conf_add_error_name_value(val); goto err; diff --git a/test/v3ext.c b/test/v3ext.c index a8ab64b2714bd..7e214cf9101d6 100644 --- 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_ex(NULL, 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 */ OPT_TEST_DECLARE_USAGE("cert.pem\n") @@ -243,6 +341,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; }