From ad15bc64b25fa67738a450053e7ba806cf78d552 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:24 +0000 Subject: [PATCH 1/27] Fix memory leak Free t allocated in cm_store_base64_to_bin() if PK11_HashBuf() fails. --- diff --git a/src/getcert.c b/src/getcert.c index e4743b0..6fe43c4 100644 --- a/src/getcert.c +++ b/src/getcert.c @@ -4084,7 +4084,9 @@ thumbprint(const char *s, SECOidTag tag, int bits) } *t++ = '\0'; } - } + } else { + free(t); + } done: free(u); return ret; From 28147cb335cfcbef9eb00715718208f39a728e37 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 2/27] Fix uninitialized value --- diff --git a/src/csrgen-o.c b/src/csrgen-o.c index 402284f..41b4f01 100644 --- a/src/csrgen-o.c +++ b/src/csrgen-o.c @@ -181,7 +181,7 @@ cm_csrgen_o_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, BIGNUM *serialbn; char buf[LINE_MAX], *s, *nickname, *pin, *password, *filename; unsigned char *extensions, *upassword, *bmp, *name, *up, *uq, md[CM_DIGEST_MAX]; - char *spkidec, *mcb64, *nows; + char *spkidec = NULL, *mcb64, *nows; const char *default_cn = CM_DEFAULT_CERT_SUBJECT_CN, *spkihex = NULL; const unsigned char *nametmp; struct tm *now; From 6fdf6f3e146e84f45947c52ffe10b0e9389fe149 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 3/27] Add all enumeration values to prefs test --- diff --git a/tests/tools/prefs.c b/tests/tools/prefs.c index f61153b..9e9f136 100644 --- a/tests/tools/prefs.c +++ b/tests/tools/prefs.c @@ -35,6 +35,9 @@ main(int argc, char **argv) case cm_prefs_aes128: printf("cipher: AES128\n"); break; + case cm_prefs_aes192: + printf("cipher: AES192\n"); + break; case cm_prefs_aes256: printf("cipher: AES256\n"); break; @@ -44,6 +47,9 @@ main(int argc, char **argv) case cm_prefs_des3: printf("cipher: DES3\n"); break; + case cm_prefs_nocipher: + printf("No cipher selected. Shouldn't happen\n"); + break; } switch (cm_prefs_preferred_digest()) { case cm_prefs_md5: @@ -61,6 +67,9 @@ main(int argc, char **argv) case cm_prefs_sha512: printf("digest: SHA512\n"); break; + case cm_prefs_nodigest: + printf("No cipher selected. Shouldn't happen\n"); + break; } if (cm_prefs_notify_ttls(&ttls, &n_ttls) == 0) { From 2175884bb171f8ff27017fdec155b79631364775 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 4/27] Add missing return value declaration to ca_get_config_file_path() --- diff --git a/src/tdbush.c b/src/tdbush.c index d1bbe4d..a10a1af 100644 --- a/src/tdbush.c +++ b/src/tdbush.c @@ -2129,6 +2129,7 @@ ca_get_serial(DBusConnection *conn, DBusMessage *msg, } /* org.fedorahosted.certonger.ca.get_config_file_path */ +static DBusHandlerResult ca_get_config_file_path(DBusConnection *conn, DBusMessage *msg, struct cm_client_info *ci, struct cm_context *ctx) { From cac3f50a3373d1fb3ecc1b0529b1487127756b8a Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 5/27] Drop const from csr and csre declaration --- diff --git a/src/dogtag.c b/src/dogtag.c index 6bb7c66..7970f58 100644 --- a/src/dogtag.c +++ b/src/dogtag.c @@ -118,9 +118,10 @@ main(int argc, const char **argv) const char *ssldir = NULL, *cainfo = NULL, *capath = NULL; const char *sslcert = NULL, *sslkey = NULL; const char *sslpin = NULL, *sslpinfile = NULL; - const char *csr = NULL, *serial = NULL, *template = NULL; + const char *serial = NULL, *template = NULL; const char *uid = NULL, *pwd = NULL, *pwdfile = NULL; const char *udn = NULL, *pin = NULL, *pinfile = NULL; + char *csr = NULL, *csre = NULL; char *poptarg; struct { char *name; From e1d8de9d6b2ba31b3096434de2abc44ef1f9c694 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 6/27] Check that sent_nonce is not NULL to avoid a potential NULL compare Theoretically if sent_nonce is NULL then the length would be 0 but don't assume this is the case. Discovered by clang. --- diff --git a/src/scep.c b/src/scep.c index 6568122..c74ca57 100644 --- a/src/scep.c +++ b/src/scep.c @@ -1039,9 +1039,9 @@ main(int argc, const char **argv) rval = CM_SUBMIT_STATUS_UNREACHABLE; goto done; } - if ((recipient_nonce_length != sent_nonce_length) || + if (sent_nonce && ((recipient_nonce_length != sent_nonce_length) || (memcmp(recipient_nonce, sent_nonce, - sent_nonce_length) != 0)) { + sent_nonce_length) != 0))) { printf(_("Error: reply nonce doesn't match request.\n")); rval = CM_SUBMIT_STATUS_UNREACHABLE; goto done; From dc7e33314b6cfb24834049ff078e12e9243b67c1 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 7/27] Make a Kerberos ccache if one doesn't already exist This was embedding the logic into a conditional to set a variable that isn't used beyond this point. Discovered by clang. --- diff --git a/src/submit-x.c b/src/submit-x.c index fa81e9a..4d9dc90 100644 --- a/src/submit-x.c +++ b/src/submit-x.c @@ -914,9 +914,8 @@ main(int argc, const char **argv) /* Maybe we need a ccache. */ if (k5 || (kpname != NULL) || (ktname != NULL)) { - if (!make_ccache || - (cm_submit_x_make_ccache(ktname, kpname, NULL) == 0)) { - k5 = TRUE; + if (!make_ccache) { + cm_submit_x_make_ccache(ktname, kpname, NULL); } } From cde4c3bf429a8e2bc7cfa2f7952f9073fab13324 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 8/27] Fix memory leak determining the dogtag operation This code scans through the state cookie to determine what operation it is executing: review, fetch or retrieve. The intermediate values used were not freed. Discovered by clang. --- diff --git a/src/dogtag.c b/src/dogtag.c index 7970f58..3f5667d 100644 --- a/src/dogtag.c +++ b/src/dogtag.c @@ -473,6 +473,8 @@ main(int argc, const char **argv) op = op_retrieve; } params = talloc_asprintf(ctx, "requestId=%s", q); + free(p); + free(q); } else { params = ""; } From fe84c918cc6ff0026e678dd08d777c6f12acf37e Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 9/27] Free the csr value after generating the request The value of the csr is being embedded into the request URI. It isn't used again. Discovered by clang. --- diff --git a/src/dogtag.c b/src/dogtag.c index 3f5667d..dc742aa 100644 --- a/src/dogtag.c +++ b/src/dogtag.c @@ -551,6 +551,7 @@ main(int argc, const char **argv) "xml=true", template, csr); + free(csr); } /* Check for creds specified as options. */ for (j = 0; j < num_soptions; j++) { From fbcf03dd44007a9b231e9396cc418a00e1a4b49a Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 10/27] Free the submit options on exit A number -o name=value pairs can be passed in. Free the memory allocated to store those name/value pairs on exit. Discovered by clang. --- diff --git a/src/dogtag.c b/src/dogtag.c index dc742aa..b0ebdac 100644 --- a/src/dogtag.c +++ b/src/dogtag.c @@ -250,6 +250,7 @@ main(int argc, const char **argv) if (strchr(poptarg, '=') == NULL) { printf(_("Submit params (-o) must be in the form of param=value.\n")); poptPrintUsage(pctx, stdout, 0); + free(soptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } soptions = realloc(soptions, @@ -261,6 +262,7 @@ main(int argc, const char **argv) p = strdup(poptarg); if (p == NULL) { printf(_("Out of memory.\n")); + free(soptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } i = strcspn(p, "="); @@ -294,6 +296,7 @@ main(int argc, const char **argv) } if (c != -1) { poptPrintUsage(pctx, stdout, 0); + free(soptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } @@ -568,6 +571,7 @@ main(int argc, const char **argv) pin = NULL; } } + free(soptions); /* Add client creds. */ if (uid != NULL) { uid = cm_submit_u_url_encode(uid); From 9f6693dadfed200e18a1b942e4243a102963da89 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 11/27] Free the approval options on exit A number -O name=value pairs can be passed in. Free the memory allocated to store those name/value pairs on exit. Discovered by clang. --- diff --git a/src/dogtag.c b/src/dogtag.c index b0ebdac..19ca7b2 100644 --- a/src/dogtag.c +++ b/src/dogtag.c @@ -227,6 +227,8 @@ main(int argc, const char **argv) if (strchr(poptarg, '=') == NULL) { printf(_("Profile params (-O) must be in the form of param=value.\n")); poptPrintUsage(pctx, stdout, 0); + free(soptions); + free(aoptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } aoptions = realloc(aoptions, @@ -238,6 +240,7 @@ main(int argc, const char **argv) p = strdup(poptarg); if (p == NULL) { printf(_("Out of memory.\n")); + free(aoptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } i = strcspn(p, "="); @@ -251,6 +254,7 @@ main(int argc, const char **argv) printf(_("Submit params (-o) must be in the form of param=value.\n")); poptPrintUsage(pctx, stdout, 0); free(soptions); + free(aoptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } soptions = realloc(soptions, @@ -297,6 +301,7 @@ main(int argc, const char **argv) if (c != -1) { poptPrintUsage(pctx, stdout, 0); free(soptions); + free(aoptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } From f3d248bbadf1f5ec53cba3eb6b3a3b4dcbc20abc Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 12/27] Free profile, issuer and/or csr on incorrect usage The values were allocated prior to exit, do proper cleanup. Discovered by clang. --- diff --git a/src/ipa.c b/src/ipa.c index 41ca908..b174034 100644 --- a/src/ipa.c +++ b/src/ipa.c @@ -771,6 +771,7 @@ main(int argc, const char **argv) printf(_("Unable to determine principal name for " "signing request.\n")); poptPrintUsage(pctx, stdout, 0); + free(reqprinc); return CM_SUBMIT_STATUS_UNCONFIGURED; } if ((profile == NULL) && @@ -813,6 +814,9 @@ main(int argc, const char **argv) CM_SUBMIT_CSR_ENV); } free(csr); + free(profile); + free(issuer); + free(reqprinc); poptPrintUsage(pctx, stdout, 0); return CM_SUBMIT_STATUS_UNCONFIGURED; } @@ -855,6 +859,10 @@ main(int argc, const char **argv) fprintf(stderr, "Error reading password from \"%s\": %s.\n", pwdfile, strerror(errno)); + free(csr); + free(profile); + free(issuer); + free(reqprinc); return CM_SUBMIT_STATUS_UNCONFIGURED; } } @@ -867,6 +875,10 @@ main(int argc, const char **argv) if ((uid != NULL) || (pwd != NULL)) { fprintf(stderr, "Both -u and -W/-w options should be specified.\n"); + free(csr); + free(profile); + free(issuer); + free(reqprinc); return CM_SUBMIT_STATUS_UNCONFIGURED; } } @@ -901,6 +913,10 @@ main(int argc, const char **argv) } } free(kerr); + free(csr); + free(profile); + free(issuer); + free(reqprinc); switch (kret) { case KRB5_KDC_UNREACH: case KRB5_REALM_CANT_RESOLVE: @@ -920,10 +936,12 @@ main(int argc, const char **argv) basedn, uid, pwd, csr, reqprinc, profile, issuer); free(csr); + free(profile); + free(issuer); + free(reqprinc); return ret; } else if (strcasecmp(mode, CM_OP_FETCH_ROOTS) == 0) { - free(csr); return fetch_roots(server, ldap_uri_cmd, ldap_uri, host, uid, pwd, domain, basedn); } From 62ef421b3443adc16fc7460d31d2609155df76e5 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 13/27] Break after finding a defaultNamingContext value There can only be a single defaultNamingContext so stop processing entries if one is found. This should theoretically never hit because there should be only one entry but just in case this will prevent leaking memory. Discovered by clang. --- diff --git a/src/ipa.c b/src/ipa.c index b174034..e429582 100644 --- a/src/ipa.c +++ b/src/ipa.c @@ -175,6 +175,11 @@ cm_find_default_naming_context(LDAP *ld, char **basedn) lmsg = ldap_next_entry(ld, lmsg)) { lbvalues = ldap_get_values_len(ld, lmsg, lncattrs[0]); + /* There should be only one defaultNamingContext so once we + * have a value we're done. */ + if (*basedn != NULL) { + break; + } if (lbvalues == NULL) { continue; } From 92796b8927d8cbb3b9cfc899d331548721abc37e Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 14/27] Fix memory leak defaults is allocated by cm_submit_d_xml_defaults() so free the value when we are done with it. Discovered by clang. --- diff --git a/src/submit-d.c b/src/submit-d.c index 149eaad..b622c84 100644 --- a/src/submit-d.c +++ b/src/submit-d.c @@ -1270,6 +1270,7 @@ restart: printf("default: %s=%s\n", p, q); } } + free(defaults); cm_submit_d_approve_result(hctx, result, &error_code, &error_reason, &error, &status, &requestId); From d087c99551cfbcea887abe7ad1d0cf645bebdf3e Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 15/27] Fix memory leak skey is duplicated, free it when done with it. Discovered by clang. --- diff --git a/src/submit-x.c b/src/submit-x.c index 4d9dc90..4a469d3 100644 --- a/src/submit-x.c +++ b/src/submit-x.c @@ -934,6 +934,7 @@ main(int argc, const char **argv) } else { cm_submit_x_add_named_arg_s(ctx, skey, sval); } + free(skey); } /* Submit the request. */ From c0567cf3f9cffc24cebebf407150508bc19c5748 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 16/27] Return NULL if realloc of CSR fails in cm_submit_u_from_file --- diff --git a/src/submit-u.c b/src/submit-u.c index b85a1bd..dca23a7 100644 --- a/src/submit-u.c +++ b/src/submit-u.c @@ -105,6 +105,9 @@ cm_submit_u_from_file(const char *filename) if (csr[length-1] != '\n') { length += 1; csr = realloc(csr, length + 1); + if (csr == NULL) { + return NULL; + } csr[length - 1] = '\n'; csr[length] = '\0'; } From 7c20afd2ffef2d4bba94ff5c513dec6dd01e0c32 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 17/27] Fix memory leak of old_keyfile in cm_certsave_o_main --- diff --git a/src/certsave-o.c b/src/certsave-o.c index 3d4018d..b145a21 100644 --- a/src/certsave-o.c +++ b/src/certsave-o.c @@ -411,6 +411,7 @@ cm_certsave_o_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, free(old_key); free(old_cert); free(next_keyfile); + free(old_keyfile); if (status != 0) { _exit(status); } From 09200f4333c832293283d1cb35c9bb6f81d35748 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 18/27] Check for NSS_Shutdown and context failures and exit This is not likely since a successful Init/Shutdown combination just happened but safety first. --- diff --git a/src/certread-n.c b/src/certread-n.c index bb61b61..3ce7ec0 100644 --- a/src/certread-n.c +++ b/src/certread-n.c @@ -158,11 +158,18 @@ cm_certread_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, _exit(status); } /* Re-open the database with modules enabled */ - NSS_ShutdownContext(ctx); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(0, "Error shutting down NSS.\n"); + _exit(1); + } ctx = NSS_InitContext(entry->cm_cert_storage_location, NULL, NULL, NULL, NULL, (readwrite ? 0 : NSS_INIT_READONLY) | NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(0, "Unable to initialize NSS.\n"); + _exit(1); + } es = util_n_fips_hook(); if (es != NULL) { cm_log(1, "Error putting NSS into FIPS mode: %s\n", es); diff --git a/src/certsave-n.c b/src/certsave-n.c index eda03b3..3518def 100644 --- a/src/certsave-n.c +++ b/src/certsave-n.c @@ -186,11 +186,18 @@ cm_certsave_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, } else { /* We don't try to force FIPS mode here, as it seems to get in * the way of saving the certificate. */ - NSS_ShutdownContext(ctx); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(0, "Error shutting down NSS.\n"); + _exit(1); + } ctx = NSS_InitContext(entry->cm_cert_storage_location, NULL, NULL, NULL, NULL, (readwrite ? 0 : NSS_INIT_READONLY) | NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(0, "Unable to initialize NSS.\n"); + _exit(1); + } /* Allocate a memory pool. */ arena = PORT_NewArena(sizeof(double)); diff --git a/src/keygen-n.c b/src/keygen-n.c index e921d7e..6832cb6 100644 --- a/src/keygen-n.c +++ b/src/keygen-n.c @@ -226,11 +226,18 @@ cm_keygen_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, break; } } - NSS_ShutdownContext(ctx); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(0, "Error shutting down NSS.\n"); + _exit(1); + } ctx = NSS_InitContext(entry->cm_key_storage_location, NULL, NULL, NULL, NULL, (readwrite ? 0 : NSS_INIT_READONLY) | NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(0, "Unable to initialize NSS.\n"); + _exit(1); + } reason = util_n_fips_hook(); if (reason != NULL) { cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); diff --git a/src/keyiread-n.c b/src/keyiread-n.c index dc1c609..b8bf353 100644 --- a/src/keyiread-n.c +++ b/src/keyiread-n.c @@ -115,11 +115,18 @@ cm_keyiread_n_get_keys(struct cm_store_entry *entry, int readwrite) break; } } - NSS_ShutdownContext(ctx); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(0, "Error shutting down NSS.\n"); + _exit(1); + } ctx = NSS_InitContext(entry->cm_key_storage_location, NULL, NULL, NULL, NULL, (readwrite ? 0 : NSS_INIT_READONLY) | NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(0, "Unable to initialize NSS.\n"); + _exit(1); + } reason = util_n_fips_hook(); if (reason != NULL) { cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); diff --git a/src/scepgen-n.c b/src/scepgen-n.c index ce73c31..440f332 100644 --- a/src/scepgen-n.c +++ b/src/scepgen-n.c @@ -183,11 +183,18 @@ cm_scepgen_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, break; } } - NSS_ShutdownContext(ctx); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(0, "Error shutting down NSS.\n"); + _exit(1); + } ctx = NSS_InitContext(entry->cm_key_storage_location, NULL, NULL, NULL, NULL, NSS_INIT_READONLY | NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(0, "Unable to initialize NSS.\n"); + _exit(1); + } reason = util_n_fips_hook(); if (reason != NULL) { cm_log(0, "Error putting NSS into FIPS mode: %s\n", reason); diff --git a/src/submit-n.c b/src/submit-n.c index f27b9c7..98fc7c5 100644 --- a/src/submit-n.c +++ b/src/submit-n.c @@ -317,11 +317,18 @@ cm_submit_n_decrypt_envelope(const unsigned char *envelope, } goto done; } - NSS_ShutdownContext(ctx); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(0, "Error shutting down NSS.\n"); + _exit(1); + } ctx = NSS_InitContext(args->entry->cm_key_storage_location, NULL, NULL, NULL, NULL, NSS_INIT_READONLY | NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(0, "Unable to initialize NSS.\n"); + _exit(1); + } reason = util_n_fips_hook(); if (reason != NULL) { cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); From c279d71173c68ede01dd5225b47172c1e94f5b6b Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 19/27] Avoid a modulo by zero range (tweight) could be 0 so check for that before getting the mod of r. --- diff --git a/src/srvloc.c b/src/srvloc.c index e8f3f5a..2269857 100644 --- a/src/srvloc.c +++ b/src/srvloc.c @@ -97,6 +97,9 @@ cm_srvloc_rand(unsigned int range) if (r < 0) { r = -r; } + if (range == 0) { + return 0; + } return r % range; } #else From 7d65601f4238e65078592b3486e47ad564f00134 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 20/27] Avoid memory overrun when adding NULL byte, fix memory leak --- diff --git a/src/getcert.c b/src/getcert.c index 6fe43c4..1450313 100644 --- a/src/getcert.c +++ b/src/getcert.c @@ -4065,7 +4065,7 @@ thumbprint(const char *s, SECOidTag tag, int bits) if (length == 0) { goto done; } - u = malloc(length); + u = malloc(length + 1); if (u == NULL) { goto done; } @@ -4086,8 +4086,10 @@ thumbprint(const char *s, SECOidTag tag, int bits) } } else { free(t); + t = NULL; } done: + free(t); free(u); return ret; } From c1287becc7731aba68ea5f8ca29bb3f9d67a32ec Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 21/27] More aoptions/soptions fixups --- diff --git a/src/dogtag.c b/src/dogtag.c index 19ca7b2..91c9c58 100644 --- a/src/dogtag.c +++ b/src/dogtag.c @@ -235,12 +235,14 @@ main(int argc, const char **argv) ++num_aoptions * sizeof(*aoptions)); if (aoptions == NULL) { printf(_("Out of memory.\n")); + free(soptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } p = strdup(poptarg); if (p == NULL) { printf(_("Out of memory.\n")); free(aoptions); + free(soptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } i = strcspn(p, "="); @@ -261,6 +263,7 @@ main(int argc, const char **argv) ++num_soptions * sizeof(*soptions)); if (soptions == NULL) { printf(_("Out of memory.\n")); + free(aoptions); return CM_SUBMIT_STATUS_UNCONFIGURED; } p = strdup(poptarg); From aee33d31c3ba2a7625caa1ca6600601ca010ac27 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 22/27] Include NSPR header so PR_PR_ErrorToString() is defined --- diff --git a/src/pkcs7.c b/src/pkcs7.c index f81174f..51b3b0f 100644 --- a/src/pkcs7.c +++ b/src/pkcs7.c @@ -38,6 +38,7 @@ #include #include #include +#include #include From 6a6962ca6bb574fe08cc0ae99767c0dc61832c88 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 23/27] Fix memory leak, always free buf --- diff --git a/src/store-files.c b/src/store-files.c index b97ba5f..4c3b223 100644 --- a/src/store-files.c +++ b/src/store-files.c @@ -559,6 +559,7 @@ cm_store_file_read_lines(void *parent, FILE *fp) break; } free(buf); + buf = NULL; } /* If we were reading a line, append it to the list. */ if (s != NULL) { @@ -573,6 +574,7 @@ cm_store_file_read_lines(void *parent, FILE *fp) lines = tlines; } } + free(buf); return lines; } From d5fd17d8121665c1546c67fea8008a908f3f9c41 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 24/27] Free XML objects on errors --- diff --git a/src/submit-d.c b/src/submit-d.c index b622c84..3adaa4a 100644 --- a/src/submit-d.c +++ b/src/submit-d.c @@ -203,6 +203,7 @@ cm_submit_d_xml_defaults(void *parent, const char *xml) ret = malloc(sizeof(*ret) * (obj->nodesetval->nodeNr + 1)); if (ret == NULL) { + xmlFree(obj); return NULL; } memset(ret, 0, @@ -358,6 +359,8 @@ cm_submit_d_xml_value_if(void *parent, xmlXPathContextPtr xpctx, v = cm_submit_d_text_node(parent, vobj); xmlXPathFreeObject(vobj); if ((v == NULL) || (strlen(v) == 0)) { + xmlFree(bpath1); + xmlFree(bpath2); return NULL; } bobj1 = NULL; From d2fde991831c2cb07808c18a55d382c03eeed56b Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 25/27] Terminate string passed to cm_pkcs7_envelope_ias in tests --- diff --git a/tests/tools/pk7env.c b/tests/tools/pk7env.c index 5a43a21..453d0e4 100644 --- a/tests/tools/pk7env.c +++ b/tests/tools/pk7env.c @@ -110,6 +110,7 @@ main(int argc, char **argv) } len += j; } + p[1][len] = '\0'; close(fd); i++; From d40c22c41c0713328c75ae5cecbb81e1f32ec61a Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 26/27] Avoid NULL dereference if strchr() returns NULL --- diff --git a/src/util.c b/src/util.c index 28e2d0d..55931aa 100644 --- a/src/util.c +++ b/src/util.c @@ -155,7 +155,11 @@ get_config_entry(char * in_data, const char *section, const char *key) /* Skip over any whitespace after the equal sign. */ line = strchr(line, '='); - line++; + if (line == NULL) { + free(data); + return NULL; + } + line++; while (isspace((unsigned char)*line) && (*line != '\0')) line++; From 63d54706be598391c3a4f580e44c5c0b2a832a03 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jun 25 2020 21:25:25 +0000 Subject: [PATCH 27/27] Don't allow unitialized value in a conditional If newcert is NULL then error is undefined (probably 0) which could lead to the code thinking the certificate imported. If the certificate wasn't decoded then set error to SECFailure --- diff --git a/src/certsave-n.c b/src/certsave-n.c index 3518def..237f4f8 100644 --- a/src/certsave-n.c +++ b/src/certsave-n.c @@ -517,6 +517,8 @@ cm_certsave_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, CK_INVALID_HANDLE, entry->cm_cert_nickname, PR_FALSE); + } else { + error = SECFailure; } if (error == SECSuccess) { cm_log(1, "Imported certificate with "