From: slontis Date: Thu, 17 Nov 2022 01:58:36 +0000 (+1000) Subject: Fix coverity issues in X509v3_addr X-Git-Tag: openssl-3.2.0-alpha1~1692 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=26cfa4cd85f6b26dd7a48c2ff06bfa4a2cea4764;p=thirdparty%2Fopenssl.git Fix coverity issues in X509v3_addr CID 1516955 : Null pointer deref (REVERSE_INULL) CID 1516954 : Null pointer deref (REVERSE_INULL) CID 1516953 : RESOURCE_LEAK of child Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/19700) --- diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c index 9b639e85aaa..0ab202400e0 100644 --- a/crypto/x509/v3_addr.c +++ b/crypto/x509/v3_addr.c @@ -1181,10 +1181,10 @@ int X509v3_addr_subset(IPAddrBlocks *a, IPAddrBlocks *b) int j = sk_IPAddressFamily_find(b, fa); IPAddressFamily *fb = sk_IPAddressFamily_value(b, j); - if (!IPAddressFamily_check_len(fa) || !IPAddressFamily_check_len(fb)) - return 0; if (fb == NULL) return 0; + if (!IPAddressFamily_check_len(fa) || !IPAddressFamily_check_len(fb)) + return 0; if (!addr_contains(fb->ipAddressChoice->u.addressesOrRanges, fa->ipAddressChoice->u.addressesOrRanges, length_from_afi(X509v3_addr_get_afi(fb)))) @@ -1202,11 +1202,11 @@ int X509v3_addr_subset(IPAddrBlocks *a, IPAddrBlocks *b) ctx->error = _err_; \ ctx->error_depth = i; \ ctx->current_cert = x; \ - ret = ctx->verify_cb(0, ctx); \ + rv = ctx->verify_cb(0, ctx); \ } else { \ - ret = 0; \ + rv = 0; \ } \ - if (!ret) \ + if (rv == 0) \ goto done; \ } while (0) @@ -1223,7 +1223,7 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, IPAddrBlocks *ext) { IPAddrBlocks *child = NULL; - int i, j, ret = 1; + int i, j, ret = 0, rv; X509 *x; if (!ossl_assert(chain != NULL && sk_X509_num(chain) > 0) @@ -1246,7 +1246,7 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, i = 0; x = sk_X509_value(chain, i); if ((ext = x->rfc3779_addr) == NULL) - goto done; + return 1; /* Return success */ } if (!X509v3_addr_is_canonical(ext)) validation_err(X509_V_ERR_INVALID_EXTENSION); @@ -1255,7 +1255,6 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, ERR_raise(ERR_LIB_X509V3, ERR_R_CRYPTO_LIB); if (ctx != NULL) ctx->error = X509_V_ERR_OUT_OF_MEM; - ret = 0; goto done; } @@ -1272,7 +1271,7 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, IPAddressFamily *fc = sk_IPAddressFamily_value(child, j); if (!IPAddressFamily_check_len(fc)) - return 0; + goto done; if (fc->ipAddressChoice->type != IPAddressChoice_inherit) { validation_err(X509_V_ERR_UNNESTED_RESOURCE); @@ -1289,9 +1288,6 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, IPAddressFamily *fp = sk_IPAddressFamily_value(x->rfc3779_addr, k); - if (!IPAddressFamily_check_len(fc) || !IPAddressFamily_check_len(fp)) - return 0; - if (fp == NULL) { if (fc->ipAddressChoice->type == IPAddressChoice_addressesOrRanges) { @@ -1300,6 +1296,10 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, } continue; } + + if (!IPAddressFamily_check_len(fc) || !IPAddressFamily_check_len(fp)) + goto done; + if (fp->ipAddressChoice->type == IPAddressChoice_addressesOrRanges) { if (fc->ipAddressChoice->type == IPAddressChoice_inherit @@ -1321,14 +1321,14 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, IPAddressFamily *fp = sk_IPAddressFamily_value(x->rfc3779_addr, j); if (!IPAddressFamily_check_len(fp)) - return 0; + goto done; if (fp->ipAddressChoice->type == IPAddressChoice_inherit && sk_IPAddressFamily_find(child, fp) >= 0) validation_err(X509_V_ERR_UNNESTED_RESOURCE); } } - + ret = 1; done: sk_IPAddressFamily_free(child); return ret; diff --git a/test/v3ext.c b/test/v3ext.c index 1f54e31f554..3cd6ee6907f 100644 --- a/test/v3ext.c +++ b/test/v3ext.c @@ -409,6 +409,49 @@ static int test_ext_syntax(void) return testresult; } + +static int test_addr_subset(void) +{ + int i; + int ret = 0; + IPAddrBlocks *addrEmpty = NULL; + IPAddrBlocks *addr[3] = { NULL, NULL }; + ASN1_OCTET_STRING *ip1[3] = { NULL, NULL }; + ASN1_OCTET_STRING *ip2[3] = { NULL, NULL }; + int sz = OSSL_NELEM(addr); + + for (i = 0; i < sz; ++i) { + /* Create the IPAddrBlocks with a good IPAddressFamily */ + if (!TEST_ptr(addr[i] = sk_IPAddressFamily_new_null()) + || !TEST_ptr(ip1[i] = a2i_IPADDRESS(ranges[i].ip1)) + || !TEST_ptr(ip2[i] = a2i_IPADDRESS(ranges[i].ip2)) + || !TEST_true(X509v3_addr_add_range(addr[i], ranges[i].afi, NULL, + ip1[i]->data, ip2[i]->data))) + goto end; + } + + ret = TEST_ptr(addrEmpty = sk_IPAddressFamily_new_null()) + && TEST_true(X509v3_addr_subset(NULL, NULL)) + && TEST_true(X509v3_addr_subset(NULL, addr[0])) + && TEST_true(X509v3_addr_subset(addrEmpty, addr[0])) + && TEST_true(X509v3_addr_subset(addr[0], addr[0])) + && TEST_true(X509v3_addr_subset(addr[0], addr[1])) + && TEST_true(X509v3_addr_subset(addr[0], addr[2])) + && TEST_true(X509v3_addr_subset(addr[1], addr[2])) + && TEST_false(X509v3_addr_subset(addr[0], NULL)) + && TEST_false(X509v3_addr_subset(addr[1], addr[0])) + && TEST_false(X509v3_addr_subset(addr[2], addr[1])) + && TEST_false(X509v3_addr_subset(addr[0], addrEmpty)); +end: + sk_IPAddressFamily_pop_free(addrEmpty, IPAddressFamily_free); + for (i = 0; i < sz; ++i) { + sk_IPAddressFamily_pop_free(addr[i], IPAddressFamily_free); + ASN1_OCTET_STRING_free(ip1[i]); + ASN1_OCTET_STRING_free(ip2[i]); + } + return ret; +} + #endif /* OPENSSL_NO_RFC3779 */ OPT_TEST_DECLARE_USAGE("cert.pem\n") @@ -429,6 +472,7 @@ int setup_tests(void) ADD_TEST(test_addr_ranges); ADD_TEST(test_ext_syntax); ADD_TEST(test_addr_fam_len); + ADD_TEST(test_addr_subset); #endif /* OPENSSL_NO_RFC3779 */ return 1; }