hx509: Fix warnings and leaks

lib/hx509/ca.c

@@ -2741,7 +2741,8 @@ set_tbs(hx509_context context,
                             realm);
 
     /* Populate requested certificate extensions from CSR/CSRPlus if allowed */
-    ret = hx509_ca_tbs_set_from_csr(context, tbs, req);
+    if (ret == 0)
+        ret = hx509_ca_tbs_set_from_csr(context, tbs, req);
     if (ret == 0)
         ret = set_template(context, logf, cf, tbs);
 
@@ -2939,6 +2940,8 @@ _hx509_ca_issue_certificate(hx509_context context,
     hx509_request_authorize_ku(req, ku);
 
     ret = get_cf(context, cf, logf, req, cprinc, &cf);
+    if (ret)
+        return ret;
 
     if ((ca = heim_config_get_string(context->hcontext, cf,
                                      "ca", NULL)) == NULL) {
@@ -3050,9 +3053,8 @@ _hx509_ca_issue_certificate(hx509_context context,
     hx509_env_free(&env);
 
     /* All done with the TBS, sign/issue the certificate */
-    ret = hx509_ca_sign(context, tbs, signer, &cert);
+    if (ret == 0)
-    if (ret)
+        ret = hx509_ca_sign(context, tbs, signer, &cert);
-        goto out;
 
     /*
      * Gather the certificate and chain into a MEMORY store, being careful not
@@ -3063,8 +3065,9 @@ _hx509_ca_issue_certificate(hx509_context context,
      * the full chain in the issuer credential store and copying only the certs
      * (but not the private keys) is safer and easier to configure.
      */
-    ret = hx509_certs_init(context, "MEMORY:certs",
+    if (ret == 0)
-                           HX509_CERTS_NO_PRIVATE_KEYS, NULL, out);
+        ret = hx509_certs_init(context, "MEMORY:certs",
+                               HX509_CERTS_NO_PRIVATE_KEYS, NULL, out);
     if (ret == 0)
         ret = hx509_certs_add(context, *out, cert);
     if (ret == 0 && send_chain) {

lib/hx509/crypto.c

@@ -1625,6 +1625,8 @@ _hx509_private_key_export(hx509_context context,
 			  hx509_key_format_t format,
 			  heim_octet_string *data)
 {
+    data->length = 0;
+    data->data = NULL;
     if (key->ops->export == NULL) {
 	hx509_clear_error_string(context);
 	return HX509_UNIMPLEMENTED_OPERATION;

lib/hx509/error.c

@@ -218,9 +218,11 @@ hx509_free_error_string(char *str)
  * @ingroup hx509_error
  */
 
-HX509_LIB_FUNCTION void HX509_LIB_CALL
+HX509_LIB_NORETURN_FUNCTION
+     __attribute__ ((__noreturn__, __format__ (__printf__, 4, 5)))
+void HX509_LIB_CALL
 hx509_err(hx509_context context, int exit_code,
-	  int error_code, const char *fmt, ...)
+          int error_code, const char *fmt, ...)
 {
     va_list ap;
     const char *msg;

lib/hx509/hxtool.c

@@ -412,17 +412,19 @@ cms_create_sd(struct cms_create_sd_options *opt, int argc, char **argv)
     size_t sz;
     void *p;
     int ret, flags = 0;
-    char *infile, *outfile = NULL;
+    const char *outfile = NULL;
+    char *infile, *freeme = NULL;
 
     memset(&contentType, 0, sizeof(contentType));
 
     infile = argv[0];
 
     if (argc < 2) {
-	ret = asprintf(&outfile, "%s.%s", infile,
+	ret = asprintf(&freeme, "%s.%s", infile,
 		       opt->pem_flag ? "pem" : "cms-signeddata");
-	if (ret == -1 || outfile == NULL)
+	if (ret == -1 || freeme == NULL)
 	    errx(1, "out of memory");
+        outfile = freeme;
     } else
 	outfile = argv[1];
 
@@ -549,6 +551,7 @@ cms_create_sd(struct cms_create_sd_options *opt, int argc, char **argv)
 
     hx509_certs_free(&signer);
     free(o.data);
+    free(freeme);
 
     return 0;
 }
@@ -843,6 +846,7 @@ pcert_validate(struct validate_options *opt, int argc, char **argv)
 	hx509_certs_iter_f(context, certs, validate_f, ctx);
 	hx509_certs_free(&certs);
 	argv++;
+        free(sn);
     }
     hx509_validate_ctx_free(ctx);
 
@@ -1363,7 +1367,7 @@ get_key(const char *fn, const char *type, int optbits,
     int ret = 0;
 
     if (type) {
-        struct hx509_generate_private_context *gen_ctx;
+        struct hx509_generate_private_context *gen_ctx = NULL;
 
 	if (strcasecmp(type, "rsa") != 0)
 	    errx(1, "can only handle rsa keys for now");
@@ -1375,6 +1379,7 @@ get_key(const char *fn, const char *type, int optbits,
             ret = _hx509_generate_private_key_bits(context, gen_ctx, optbits);
         if (ret == 0)
             ret = _hx509_generate_private_key(context, gen_ctx, signer);
+        _hx509_generate_private_key_free(&gen_ctx);
         if (ret)
             hx509_err(context, 1, ret, "failed to generate private key of type %s", type);
 
@@ -1420,6 +1425,7 @@ generate_key(struct generate_key_options *opt, int argc, char **argv)
     const char *type = opt->type_string ? opt->type_string : "rsa";
     int bits = opt->key_bits_integer ? opt->key_bits_integer : 2048;
 
+    memset(&signer, 0, sizeof(signer));
     get_key(argv[0], type, bits, &signer);
     hx509_private_key_free(&signer);
     return 0;
@@ -1436,6 +1442,7 @@ request_create(struct request_create_options *opt, int argc, char **argv)
     const char *outfile = argv[0];
 
     memset(&key, 0, sizeof(key));
+    memset(&signer, 0, sizeof(signer));
 
     get_key(opt->key_string,
 	    opt->generate_key_string,
@@ -2507,6 +2514,7 @@ crl_sign(struct crl_sign_options *opt, int argc, char **argv)
 	    ret = hx509_certs_append(context, revoked, lock, sn);
 	    if (ret)
 		hx509_err(context, 1, ret, "hx509_certs_append: %s", sn);
+            free(sn);
 	}
 
 	hx509_crl_add_revoked_certs(context, crl, revoked);
@@ -2983,7 +2991,7 @@ acert1(struct acert_options *opt, size_t cert_num, hx509_cert cert, int *matched
     ekus_wanted = opt->has_eku_strings.num_strings;
     kus_wanted = opt->has_ku_strings.num_strings;
     wanted = sans_wanted + ekus_wanted + kus_wanted;
-    found = sans_found = ekus_found = kus_found = 0;
+    sans_found = ekus_found = kus_found = 0;
 
     if (e == NULL) {
         if (wanted)
@@ -3080,6 +3088,8 @@ acert(struct acert_options *opt, int argc, char **argv)
             ret = acert1(opt, n++, cert, &matched);
             if (matched)
                 break;
+            hx509_cert_free(cert);
+            cert = NULL;
         }
         if (cursor)
             (void) hx509_certs_end_seq(context, certs, cursor);
@@ -3093,6 +3103,7 @@ acert(struct acert_options *opt, int argc, char **argv)
     if (ret)
         hx509_err(context, 1, ret, "Matching certificate did not meet "
                   "requirements");
+    hx509_cert_free(cert);
     free(sn);
     return 0;
 }

lib/hx509/keyset.c

@@ -561,11 +561,14 @@ hx509_certs_find(hx509_context context,
 	    break;
 	if (_hx509_query_match_cert(context, q, c)) {
 	    *r = c;
+            c = NULL;
 	    break;
 	}
 	hx509_cert_free(c);
+        c = NULL;
     }
 
+    hx509_cert_free(c);
     hx509_certs_end_seq(context, certs, cursor);
     if (ret)
 	return ret;
@@ -573,7 +576,7 @@ hx509_certs_find(hx509_context context,
      * Return HX509_CERT_NOT_FOUND if no certificate in certs matched
      * the query.
      */
-    if (c == NULL) {
+    if (*r == NULL) {
 	hx509_clear_error_string(context);
 	return HX509_CERT_NOT_FOUND;
     }

lib/hx509/ks_file.c

@@ -548,7 +548,7 @@ store_func(hx509_context context, void *ctx, hx509_cert c)
 {
     struct store_ctx *sc = ctx;
     heim_octet_string data;
-    int ret;
+    int ret = 0;
 
     if (hx509_cert_have_private_key_only(c)) {
         data.length = 0;
@@ -564,15 +564,17 @@ store_func(hx509_context context, void *ctx, hx509_cert c)
         /* Can't store both.  Well, we could, but nothing will support it */
         if (data.data) {
             fwrite(data.data, data.length, 1, sc->f);
-            free(data.data);
         } else if (_hx509_cert_private_key_exportable(c) &&
                    !(sc->store_flags & HX509_CERTS_STORE_NO_PRIVATE_KEYS)) {
             hx509_private_key key = _hx509_cert_private_key(c);
 
+            free(data.data);
+            data.length = 0;
+            data.data = NULL;
             ret = _hx509_private_key_export(context, key,
                                             HX509_KEY_FORMAT_DER, &data);
-            fwrite(data.data, data.length, 1, sc->f);
+            if (ret == 0 && data.length)
-            free(data.data);
+                fwrite(data.data, data.length, 1, sc->f);
         }
 	break;
     case USE_PEM:
@@ -583,23 +585,20 @@ store_func(hx509_context context, void *ctx, hx509_cert c)
 
 	    ret = _hx509_private_key_export(context, key,
 					    HX509_KEY_FORMAT_DER, &priv_key);
-	    if (ret) {
+            if (ret == 0)
-                free(data.data);
+                ret = hx509_pem_write(context, _hx509_private_pem_name(key), NULL,
-		break;
+                                      sc->f, priv_key.data, priv_key.length);
-            }
-	    hx509_pem_write(context, _hx509_private_pem_name(key), NULL, sc->f,
-			    priv_key.data, priv_key.length);
 	    free(priv_key.data);
 	}
-        if (data.data) {
+        if (ret == 0 && data.data) {
-            hx509_pem_write(context, "CERTIFICATE", NULL, sc->f,
+            ret = hx509_pem_write(context, "CERTIFICATE", NULL, sc->f,
-                            data.data, data.length);
+                                  data.data, data.length);
-            free(data.data);
         }
 	break;
     }
 
-    return 0;
+    free(data.data);
+    return ret;
 }
 
 static int

lib/hx509/name.c

@@ -358,29 +358,29 @@ _hx509_Name_to_string(const Name *n, char **str)
     return 0;
 }
 
-#define COPYCHARARRAY(_ds,_el,_l,_n)		\
+#define COPYCHARARRAY(_ds,_el,_l,_n)			\
-        (_l) = strlen(_ds->u._el);		\
+        (_l) = strlen(_ds->u._el);			\
-	(_n) = malloc((_l) * sizeof((_n)[0]));	\
+	(_n) = malloc((_l + 1) * sizeof((_n)[0]));	\
-	if ((_n) == NULL)			\
+	if ((_n) == NULL)				\
-	    return ENOMEM;			\
+	    return ENOMEM;				\
-	for (i = 0; i < (_l); i++)		\
+	for (i = 0; i < (_l); i++)			\
 	    (_n)[i] = _ds->u._el[i]
 
 
-#define COPYVALARRAY(_ds,_el,_l,_n)		\
+#define COPYVALARRAY(_ds,_el,_l,_n)			\
-        (_l) = _ds->u._el.length;		\
+        (_l) = _ds->u._el.length;			\
-	(_n) = malloc((_l) * sizeof((_n)[0]));	\
+	(_n) = malloc((_l + 1) * sizeof((_n)[0]));	\
-	if ((_n) == NULL)			\
+	if ((_n) == NULL)				\
-	    return ENOMEM;			\
+	    return ENOMEM;				\
-	for (i = 0; i < (_l); i++)		\
+	for (i = 0; i < (_l); i++)			\
 	    (_n)[i] = _ds->u._el.data[i]
 
-#define COPYVOIDARRAY(_ds,_el,_l,_n)		\
+#define COPYVOIDARRAY(_ds,_el,_l,_n)			\
-        (_l) = _ds->u._el.length;		\
+        (_l) = _ds->u._el.length;			\
-	(_n) = malloc((_l) * sizeof((_n)[0]));	\
+	(_n) = malloc((_l + 1) * sizeof((_n)[0]));	\
-	if ((_n) == NULL)			\
+	if ((_n) == NULL)				\
-	    return ENOMEM;			\
+	    return ENOMEM;				\
-	for (i = 0; i < (_l); i++)		\
+	for (i = 0; i < (_l); i++)			\
 	    (_n)[i] = ((unsigned char *)_ds->u._el.data)[i]
 
 
@@ -423,7 +423,7 @@ dsstringprep(const DirectoryString *ds, uint32_t **rname, size_t *rlen)
 	ret = wind_utf8ucs4_length(ds->u.utf8String, &len);
 	if (ret)
 	    return ret;
-	name = malloc(len * sizeof(name[0]));
+	name = malloc((len + 1) * sizeof(name[0]));
 	if (name == NULL)
 	    return ENOMEM;
 	ret = wind_utf8ucs4(ds->u.utf8String, name, &len);
@@ -440,7 +440,7 @@ dsstringprep(const DirectoryString *ds, uint32_t **rname, size_t *rlen)
     /* try a couple of times to get the length right, XXX gross */
     for (i = 0; i < 4; i++) {
 	*rlen = *rlen * 2;
-	if ((*rname = malloc(*rlen * sizeof((*rname)[0]))) == NULL) {
+	if ((*rname = malloc((rlen[0] + 1) * sizeof((*rname)[0]))) == NULL) {
             ret = ENOMEM;
             break;
         }
@@ -579,9 +579,9 @@ _hx509_name_modify(hx509_context context,
 {
     RelativeDistinguishedName rdn;
     size_t max_len = oidtomaxlen(oid);
-    int type_choice, ret;
-    const char *a = oidtostring(oid, &type_choice);
     char *s = NULL;
+    int type_choice = choice_DirectoryString_printableString;
+    int ret;
 
     /*
      * Check string length upper bounds.
@@ -591,10 +591,13 @@ _hx509_name_modify(hx509_context context,
      * here.
      */
     if (max_len && strlen(str) > max_len) {
+        char *a = oidtostring(oid, &type_choice);
+
         ret = HX509_PARSING_NAME_FAILED;
         hx509_set_error_string(context, 0, ret, "RDN attribute %s value too "
                                "long (max %llu): %s", a ? a : "<unknown>",
                                max_len, str);
+        free(a);
         return ret;
     }
 
@@ -622,7 +625,7 @@ _hx509_name_modify(hx509_context context,
      */
     rdn.val[0].value.element = type_choice;
     if ((s = strdup(str)) == NULL ||
-        (ret = der_copy_oid(oid, &rdn.val[0].type))) {
+        der_copy_oid(oid, &rdn.val[0].type)) {
         free(rdn.val);
         free(s);
 	return hx509_enomem(context);
@@ -1392,7 +1395,9 @@ hx509_general_name_unparse(GeneralName *name, char **str)
 
     if ((ret = hx509_context_init(&context)))
         return ret;
-    return hx509_general_name_unparse2(context, name, str);
+    ret = hx509_general_name_unparse2(context, name, str);
+    hx509_context_free(&context);
+    return ret;
 }
 
 /**
@@ -1511,8 +1516,9 @@ hx509_general_name_unparse2(hx509_context context,
     default:
 	return EINVAL;
     }
-    if (strpool == NULL ||
+    if (ret)
-        (*str = rk_strpoolcollect(strpool)) == NULL)
+        rk_strpoolfree(strpool);
+    else if (strpool == NULL || (*str = rk_strpoolcollect(strpool)) == NULL)
 	return ENOMEM;
-    return 0;
+    return ret;
 }

lib/hx509/print.c

@@ -361,6 +361,7 @@ check_authorityKeyIdentifier(hx509_validate_ctx ctx,
 	}
     }
 
+    free_AuthorityKeyIdentifier(&ai);
     return 0;
 }
 
@@ -771,6 +772,7 @@ check_certificatePolicies(hx509_validate_ctx ctx,
                         validate_print(ctx, HX509_VALIDATE_F_VERBOSE,
                                        " Unknown:%s", qoid);
                     }
+                    free_UserNotice(&un);
                 }
             } else {
                 validate_print(ctx, HX509_VALIDATE_F_VERBOSE,
@@ -842,8 +844,11 @@ check_policyMappings(hx509_validate_ctx ctx,
         else
             validate_print(ctx, HX509_VALIDATE_F_VALIDATE,
                            "ret=%d while decoding PolicyMappings\n", ret);
+        free(sdpoid);
+        free(idpoid);
     }
 
+    free_PolicyMappings(&pm);
     return 0;
 }
 

lib/hx509/req.c

@@ -518,14 +518,13 @@ get_exts(hx509_context context,
          const hx509_request req,
          Extensions *exts)
 {
-    uint64_t ku_num;
     size_t size;
     int ret = 0;
 
     exts->val = NULL;
     exts->len = 0;
 
-    if ((ku_num = KeyUsage2int(req->ku))) {
+    if (KeyUsage2int(req->ku)) {
         Extension e;
 
         memset(&e, 0, sizeof(e));
@@ -718,6 +717,7 @@ hx509_request_to_pkcs10(hx509_context context,
 	abort();
 
     free_CertificationRequest(&r);
+    free_Extensions(&exts);
     return ret;
 }
 
@@ -899,9 +899,9 @@ hx509_request_parse_der(hx509_context context,
 
 out:
     free_CertificationRequest(&r);
+    free_Extensions(&exts);
     if (ret)
         hx509_request_free(req);
-    free_CertificationRequest(&r);
     return ret;
 }
 

lib/hx509/softp11.c

@@ -311,7 +311,7 @@ add_st_object(void)
 	return NULL;
 
     for (i = 0; i < soft_token.object.num_objs; i++) {
-	if (soft_token.object.objs == NULL) {
+	if (soft_token.object.objs[i] == NULL) {
 	    soft_token.object.objs[i] = o;
 	    break;
 	}
@@ -342,6 +342,9 @@ add_object_attribute(struct st_object *o,
     struct st_attr *a;
     int i;
 
+    if (pValue == NULL && ulValueLen)
+        return CKR_ARGUMENTS_BAD;
+
     i = o->num_attributes;
     a = realloc(o->attrs, (i + 1) * sizeof(o->attrs[0]));
     if (a == NULL)
@@ -352,7 +355,8 @@ add_object_attribute(struct st_object *o,
     o->attrs[i].attribute.pValue = malloc(ulValueLen);
     if (o->attrs[i].attribute.pValue == NULL && ulValueLen != 0)
 	return CKR_DEVICE_MEMORY;
-    memcpy(o->attrs[i].attribute.pValue, pValue, ulValueLen);
+    if (ulValueLen)
+        memcpy(o->attrs[i].attribute.pValue, pValue, ulValueLen);
     o->attrs[i].attribute.ulValueLen = ulValueLen;
     o->num_attributes++;