Skip to content

Commit

Permalink
Fix coverity issues in X509v3_addr
Browse files Browse the repository at this point in the history 
CID 1516955 : Null pointer deref (REVERSE_INULL)
CID 1516954 : Null pointer deref (REVERSE_INULL)
CID 1516953 : RESOURCE_LEAK of child

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #19700)
  • Loading branch information
@slontis @t8m
slontis authored and t8m committed Nov 21, 2022
1 parent 4741c80 commit 26cfa4c
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 14 deletions.
28 changes: 14 additions & 14 deletions crypto/x509/v3_addr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))))
Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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;
Expand Down
44 changes: 44 additions & 0 deletions test/v3ext.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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;
}

0 comments on commit 26cfa4c

Please sign in to comment.