Skip to content

Commit

Permalink
Fix a crash in v2i_IPAddrBlocks()
Browse files Browse the repository at this point in the history 
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>
(Merged from #18523)
  • Loading branch information
@mattcaswell
mattcaswell committed Jul 22, 2022
1 parent 2752ab2 commit b91ad3c
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 5 deletions.
16 changes: 11 additions & 5 deletions crypto/x509/v3_addr.c
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
99 changes: 99 additions & 0 deletions test/v3ext.c
View file
Expand Up @@ -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")
Expand All @@ -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;
}

0 comments on commit b91ad3c

Please sign in to comment.