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 <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19700)

diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c
index 9b639e85aaa134bcc957c82a211081c0ef1ba725..0ab202400e053a5ab4d56dcd7afaecdc5b49ea80 100644 (file)
--- 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 1f54e31f554c02cbb13f75158f8059575237a8cd..3cd6ee6907f369be4392949b3f4f315d90c7955e 100644 (file)
--- 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;
 }