From dd71303a2feee0e54a7fe21589e10af767425a70 Mon Sep 17 00:00:00 2001
From: Nicolas Williams <nico@twosigma.com>
Date: Mon, 17 Jan 2022 17:36:48 -0600
Subject: [PATCH] hx509: Fix coverity warnings

---
 lib/hx509/ca.c        |  4 ++--
 lib/hx509/cert.c      | 32 +++++++++++++++++++++++---------
 lib/hx509/cms.c       |  4 +++-
 lib/hx509/collector.c |  3 ++-
 lib/hx509/file.c      |  6 +-----
 lib/hx509/hxtool.c    |  5 ++++-
 lib/hx509/revoke.c    |  4 ++++
 7 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/lib/hx509/ca.c b/lib/hx509/ca.c
index ac0fc56b3..3d62b93fa 100644
--- a/lib/hx509/ca.c
+++ b/lib/hx509/ca.c
@@ -2565,9 +2565,9 @@ get_cf(hx509_context context,
     }
 
     *out = heim_config_get_list(context->hcontext, cf, label, svc, NULL);
-    if (*out)
+    if (*out) {
         ret = 0;
-    if (ret) {
+    } else {
         heim_log_msg(context->hcontext, logf, 3, NULL,
                      "No configuration for %s %s certificate's realm "
                      "-> %s -> kx509 -> %s%s%s", def, label, realm, label,
diff --git a/lib/hx509/cert.c b/lib/hx509/cert.c
index 9cff047e3..59c9136df 100644
--- a/lib/hx509/cert.c
+++ b/lib/hx509/cert.c
@@ -893,9 +893,12 @@ HX509_LIB_FUNCTION void HX509_LIB_CALL
 hx509_free_octet_string_list(hx509_octet_string_list *list)
 {
     size_t i;
-    for (i = 0; i < list->len; i++)
-	der_free_octet_string(&list->val[i]);
-    free(list->val);
+
+    if (list->val) {
+        for (i = 0; i < list->len; i++)
+            der_free_octet_string(&list->val[i]);
+        free(list->val);
+    }
     list->val = NULL;
     list->len = 0;
 }
@@ -2809,6 +2812,12 @@ _hx509_set_cert_attribute(hx509_context context,
 {
     hx509_cert_attribute a;
     void *d;
+    int ret;
+
+    /*
+     * TODO: Rewrite this (and hx509_cert_attribute, and _hx509_cert_attrs) to
+     * use the add_AttributeValues() util generated by asn1_compile.
+     */
 
     if (hx509_cert_get_attribute(cert, oid) != NULL)
 	return 0;
@@ -2825,13 +2834,18 @@ _hx509_set_cert_attribute(hx509_context context,
     if (a == NULL)
 	return ENOMEM;
 
-    der_copy_octet_string(attr, &a->data);
-    der_copy_oid(oid, &a->oid);
+    ret = der_copy_octet_string(attr, &a->data);
+    if (ret == 0)
+        ret = der_copy_oid(oid, &a->oid);
+    if (ret == 0) {
+        cert->attrs.val[cert->attrs.len] = a;
+        cert->attrs.len++;
+    } else {
+        der_free_octet_string(&a->data);
+        free(a);
+    }
 
-    cert->attrs.val[cert->attrs.len] = a;
-    cert->attrs.len++;
-
-    return 0;
+    return ret;
 }
 
 /**
diff --git a/lib/hx509/cms.c b/lib/hx509/cms.c
index c38c2b98d..2ebbe8622 100644
--- a/lib/hx509/cms.c
+++ b/lib/hx509/cms.c
@@ -1566,7 +1566,9 @@ hx509_cms_create_signed(hx509_context context,
 
     sigctx.sd.version = cMSVersion_v3;
 
-    der_copy_oid(eContentType, &sigctx.sd.encapContentInfo.eContentType);
+    ret = der_copy_oid(eContentType, &sigctx.sd.encapContentInfo.eContentType);
+    if (ret)
+        goto out;
 
     /**
      * Use HX509_CMS_SIGNATURE_DETACHED to create detached signatures.
diff --git a/lib/hx509/collector.c b/lib/hx509/collector.c
index dd6222687..7b4680981 100644
--- a/lib/hx509/collector.c
+++ b/lib/hx509/collector.c
@@ -191,8 +191,9 @@ match_localkeyid(hx509_context context,
     q.local_key_id = &value->localKeyId;
 
     ret = hx509_certs_find(context, certs, &q, &cert);
+    if (ret == 0 && cert == NULL)
+        ret = HX509_CERT_NOT_FOUND;
     if (ret == 0) {
-
 	if (value->private_key)
 	    _hx509_cert_assign_key(cert, value->private_key);
 	hx509_cert_free(cert);
diff --git a/lib/hx509/file.c b/lib/hx509/file.c
index cd45af3ae..a3caa372a 100644
--- a/lib/hx509/file.c
+++ b/lib/hx509/file.c
@@ -350,12 +350,8 @@ _hx509_erase_file(hx509_context context, const char *fn)
 
     fd = open(fn, O_RDWR | O_BINARY | O_CLOEXEC | O_NOFOLLOW);
     if (fd < 0)
-	return errno;
+	return errno == ENOENT ? 0 : errno;
     rk_cloexec(fd);
-    if (ret == -1 && errno == ENOENT)
-        return 0;
-    if (ret == -1)
-        return errno;
 
     if (unlink(fn) < 0) {
 	ret = errno;
diff --git a/lib/hx509/hxtool.c b/lib/hx509/hxtool.c
index 0714fccbd..1bcfdfa44 100644
--- a/lib/hx509/hxtool.c
+++ b/lib/hx509/hxtool.c
@@ -2785,9 +2785,12 @@ acert1_kus(struct acert_options *opt,
     size_t unwanted = 0;
     size_t wanted = opt->has_ku_strings.num_strings;
     size_t i, k, sz;
+    int ret;
 
     memset(&ku, 0, sizeof(ku));
-    decode_KeyUsage(e->extnValue.data, e->extnValue.length, &ku, &sz);
+    ret = decode_KeyUsage(e->extnValue.data, e->extnValue.length, &ku, &sz);
+    if (ret)
+        return ret;
     ku_num = KeyUsage2int(ku);
 
     /* Validate requested key usage values */
diff --git a/lib/hx509/revoke.c b/lib/hx509/revoke.c
index c2f2e00cc..18b2f8f8f 100644
--- a/lib/hx509/revoke.c
+++ b/lib/hx509/revoke.c
@@ -202,6 +202,8 @@ verify_ocsp(hx509_context context,
     ret = hx509_certs_find(context, certs, &q, &signer);
     if (ret && ocsp->certs)
 	ret = hx509_certs_find(context, ocsp->certs, &q, &signer);
+    if (ret == 0 && signer == NULL)
+        ret = HX509_CERT_NOT_FOUND;
     if (ret)
 	goto out;
 
@@ -500,6 +502,8 @@ verify_crl(hx509_context context,
 	q.subject_name = &crl->tbsCertList.issuer;
 
 	ret = hx509_certs_find(context, certs, &q, &signer);
+        if (ret == 0 && signer == NULL)
+            ret = HX509_CERT_NOT_FOUND;
 	if (ret) {
 	    hx509_set_error_string(context, HX509_ERROR_APPEND, ret,
 				   "Failed to find certificate for CRL");