diff --git a/Fix-many-unlikely-memory-leaks.patch b/Fix-many-unlikely-memory-leaks.patch new file mode 100644 index 0000000000000000000000000000000000000000..d5b4ae0f5f77ce3ef6623a2ba8517bbdfebb77f7 --- /dev/null +++ b/Fix-many-unlikely-memory-leaks.patch @@ -0,0 +1,1054 @@ +From a06945b4ec267e8b80e5e8c95edd89930ff12103 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 26 May 2021 17:35:06 -0400 +Subject: [PATCH] Fix many unlikely memory leaks + +These are on error paths and often require allocation failures, so are +unlikely to be issues in practice. Reported by Coverity and cppcheck. +--- + src/kadmin/dbutil/kadm5_create.c | 19 +++--- + src/kadmin/dbutil/kdb5_create.c | 23 +++---- + src/kadmin/ktutil/ktutil_funcs.c | 3 +- + src/kdc/do_tgs_req.c | 27 ++++---- + src/kprop/kpropd.c | 26 ++++---- + src/kprop/kproplog.c | 25 ++++--- + src/lib/gssapi/spnego/spnego_mech.c | 6 +- + src/lib/kadm5/srv/pwqual_dict.c | 11 +++- + src/lib/kdb/kdb5.c | 5 +- + src/lib/kdb/kdb_convert.c | 65 +++++++++++-------- + src/lib/krad/remote.c | 7 +- + src/lib/krb5/krb/gic_keytab.c | 3 +- + src/lib/krb5/krb/s4u_authdata.c | 4 +- + src/lib/krb5/krb/send_tgs.c | 6 +- + src/lib/krb5/os/dnssrv.c | 2 +- + src/lib/krb5/os/sendto_kdc.c | 4 +- + src/plugins/audit/kdc_j_encode.c | 5 +- + src/plugins/kdb/db2/adb_policy.c | 10 +-- + src/plugins/kdb/db2/kdb_db2.c | 2 +- + .../kdb/ldap/libkdb_ldap/ldap_principal.c | 14 ++-- + .../kdb/ldap/libkdb_ldap/ldap_principal2.c | 8 +-- + .../preauth/pkinit/pkinit_crypto_openssl.c | 58 ++++++++++------- + src/plugins/preauth/pkinit/pkinit_identity.c | 10 +-- + src/util/profile/prof_get.c | 13 ++-- + 24 files changed, 198 insertions(+), 158 deletions(-) + +diff --git a/src/kadmin/dbutil/kadm5_create.c b/src/kadmin/dbutil/kadm5_create.c +index 42b45aa2d..db12cac36 100644 +--- a/src/kadmin/dbutil/kadm5_create.c ++++ b/src/kadmin/dbutil/kadm5_create.c +@@ -185,21 +185,24 @@ static int add_admin_princs(void *handle, krb5_context context, char *realm) + int add_admin_princ(void *handle, krb5_context context, + char *name, char *realm, int attrs, int lifetime) + { +- char *fullname; ++ char *fullname = NULL; + krb5_error_code ret; + kadm5_principal_ent_rec ent; + long flags; ++ int fret; + + memset(&ent, 0, sizeof(ent)); + + if (asprintf(&fullname, "%s@%s", name, realm) < 0) { + com_err(progname, ENOMEM, _("while appending realm to principal")); +- return ERR; ++ fret = ERR; ++ goto cleanup; + } + ret = krb5_parse_name(context, fullname, &ent.principal); + if (ret) { + com_err(progname, ret, _("while parsing admin principal name")); +- return(ERR); ++ fret = ERR; ++ goto cleanup; + } + ent.max_life = lifetime; + ent.attributes = attrs; +@@ -210,13 +213,13 @@ int add_admin_princ(void *handle, krb5_context context, + ret = kadm5_create_principal(handle, &ent, flags, NULL); + if (ret && ret != KADM5_DUP) { + com_err(progname, ret, _("while creating principal %s"), fullname); +- krb5_free_principal(context, ent.principal); +- free(fullname); +- return ERR; ++ fret = ERR; ++ goto cleanup; + } + ++ fret = OK; ++cleanup: + krb5_free_principal(context, ent.principal); + free(fullname); +- +- return OK; ++ return fret; + } +diff --git a/src/kadmin/dbutil/kdb5_create.c b/src/kadmin/dbutil/kdb5_create.c +index efdb8adb0..f9205f84d 100644 +--- a/src/kadmin/dbutil/kdb5_create.c ++++ b/src/kadmin/dbutil/kdb5_create.c +@@ -393,7 +393,7 @@ add_principal(context, princ, op, pblock) + struct realm_info *pblock; + { + krb5_error_code retval; +- krb5_db_entry *entry; ++ krb5_db_entry *entry = NULL; + krb5_kvno mkey_kvno; + krb5_timestamp now; + struct iterate_args iargs; +@@ -410,20 +410,20 @@ add_principal(context, princ, op, pblock) + entry->expiration = pblock->expiration; + + if ((retval = krb5_copy_principal(context, princ, &entry->princ))) +- goto error_out; ++ goto cleanup; + + if ((retval = krb5_timeofday(context, &now))) +- goto error_out; ++ goto cleanup; + + if ((retval = krb5_dbe_update_mod_princ_data(context, entry, + now, &db_create_princ))) +- goto error_out; ++ goto cleanup; + + switch (op) { + case MASTER_KEY: + if ((entry->key_data=(krb5_key_data*)malloc(sizeof(krb5_key_data))) + == NULL) +- goto error_out; ++ goto cleanup; + memset(entry->key_data, 0, sizeof(krb5_key_data)); + entry->n_key_data = 1; + +@@ -435,7 +435,7 @@ add_principal(context, princ, op, pblock) + if ((retval = krb5_dbe_encrypt_key_data(context, pblock->key, + &master_keyblock, NULL, + mkey_kvno, entry->key_data))) +- return retval; ++ goto cleanup; + /* + * There should always be at least one "active" mkey so creating the + * KRB5_TL_ACTKVNO entry now so the initial mkey is active. +@@ -445,11 +445,11 @@ add_principal(context, princ, op, pblock) + /* earliest possible time in case system clock is set back */ + actkvno.act_time = 0; + if ((retval = krb5_dbe_update_actkvno(context, entry, &actkvno))) +- return retval; ++ goto cleanup; + + /* so getprinc shows the right kvno */ + if ((retval = krb5_dbe_update_mkvno(context, entry, mkey_kvno))) +- return retval; ++ goto cleanup; + + break; + case TGT_KEY: +@@ -464,10 +464,11 @@ add_principal(context, princ, op, pblock) + 1, + tgt_keysalt_iterate, + (krb5_pointer) &iargs))) +- return retval; ++ goto cleanup; + break; + case NULL_KEY: +- return EOPNOTSUPP; ++ retval = EOPNOTSUPP; ++ goto cleanup; + default: + break; + } +@@ -479,7 +480,7 @@ add_principal(context, princ, op, pblock) + + retval = krb5_db_put_principal(context, entry); + +-error_out: ++cleanup: + krb5_db_free_principal(context, entry); + return retval; + } +diff --git a/src/kadmin/ktutil/ktutil_funcs.c b/src/kadmin/ktutil/ktutil_funcs.c +index e2e005d22..56bed1bbc 100644 +--- a/src/kadmin/ktutil/ktutil_funcs.c ++++ b/src/kadmin/ktutil/ktutil_funcs.c +@@ -256,7 +256,8 @@ krb5_error_code ktutil_add(context, list, princ_str, fetch, kvno, + *last = lp; + + cleanup: +- krb5_kt_free_entry(context, entry); ++ krb5_free_keytab_entry_contents(context, entry); ++ free(entry); + zapfree(password.data, password.length); + krb5_free_data_contents(context, &salt); + krb5_free_data_contents(context, ¶ms); +diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c +index 6d244ffd4..582e497cc 100644 +--- a/src/kdc/do_tgs_req.c ++++ b/src/kdc/do_tgs_req.c +@@ -162,17 +162,14 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt, + } + + errcode = kdc_make_rstate(kdc_active_realm, &state); +- if (errcode !=0) { +- krb5_free_kdc_req(kdc_context, request); +- return errcode; +- } ++ if (errcode != 0) ++ goto cleanup; + + /* Initialize audit state. */ + errcode = kau_init_kdc_req(kdc_context, request, from, &au_state); +- if (errcode) { +- krb5_free_kdc_req(kdc_context, request); +- return errcode; +- } ++ if (errcode) ++ goto cleanup; ++ + /* Seed the audit trail with the request ID and basic information. */ + kau_tgs_req(kdc_context, TRUE, au_state); + +@@ -733,11 +730,13 @@ cleanup: + if (errcode) + emsg = krb5_get_error_message (kdc_context, errcode); + +- au_state->status = status; +- if (!errcode) +- au_state->reply = &reply; +- kau_tgs_req(kdc_context, errcode ? FALSE : TRUE, au_state); +- kau_free_kdc_req(au_state); ++ if (au_state != NULL) { ++ au_state->status = status; ++ if (!errcode) ++ au_state->reply = &reply; ++ kau_tgs_req(kdc_context, errcode ? FALSE : TRUE, au_state); ++ kau_free_kdc_req(au_state); ++ } + + log_tgs_req(kdc_context, from, request, &reply, cprinc, + sprinc, altcprinc, authtime, +@@ -747,7 +746,7 @@ cleanup: + emsg = NULL; + } + +- if (errcode) { ++ if (errcode && state != NULL) { + int got_err = 0; + if (status == 0) { + status = krb5_get_error_message (kdc_context, errcode); +diff --git a/src/kprop/kpropd.c b/src/kprop/kpropd.c +index 498ca599a..46bb9d1f7 100644 +--- a/src/kprop/kpropd.c ++++ b/src/kprop/kpropd.c +@@ -632,7 +632,7 @@ krb5_error_code + do_iprop() + { + kadm5_ret_t retval; +- krb5_principal iprop_svc_principal; ++ krb5_principal iprop_svc_principal = NULL; + void *server_handle = NULL; + char *iprop_svc_princstr = NULL, *primary_svc_princstr = NULL; + unsigned int pollin, backoff_time; +@@ -652,16 +652,13 @@ do_iprop() + if (pollin == 0) + pollin = 10; + +- if (primary_svc_princstr == NULL) { +- retval = kadm5_get_kiprop_host_srv_name(kpropd_context, realm, +- &primary_svc_princstr); +- if (retval) { +- com_err(progname, retval, +- _("%s: unable to get kiprop host based " +- "service name for realm %s\n"), +- progname, realm); +- return retval; +- } ++ retval = kadm5_get_kiprop_host_srv_name(kpropd_context, realm, ++ &primary_svc_princstr); ++ if (retval) { ++ com_err(progname, retval, _("%s: unable to get kiprop host based " ++ "service name for realm %s\n"), ++ progname, realm); ++ goto done; + } + + retval = sn2princ_realm(kpropd_context, NULL, KIPROP_SVC_NAME, realm, +@@ -669,7 +666,7 @@ do_iprop() + if (retval) { + com_err(progname, retval, + _("while trying to construct host service principal")); +- return retval; ++ goto done; + } + + retval = krb5_unparse_name(kpropd_context, iprop_svc_principal, +@@ -677,10 +674,8 @@ do_iprop() + if (retval) { + com_err(progname, retval, + _("while canonicalizing principal name")); +- krb5_free_principal(kpropd_context, iprop_svc_principal); +- return retval; ++ goto done; + } +- krb5_free_principal(kpropd_context, iprop_svc_principal); + + reinit: + /* +@@ -995,6 +990,7 @@ error: + done: + free(iprop_svc_princstr); + free(primary_svc_princstr); ++ krb5_free_principal(kpropd_context, iprop_svc_principal); + krb5_free_default_realm(kpropd_context, def_realm); + kadm5_destroy(server_handle); + krb5_db_fini(kpropd_context); +diff --git a/src/kprop/kproplog.c b/src/kprop/kproplog.c +index 0c025f0eb..9d3a91070 100644 +--- a/src/kprop/kproplog.c ++++ b/src/kprop/kproplog.c +@@ -398,28 +398,36 @@ print_update(kdb_hlog_t *ulog, uint32_t entry, uint32_t ulogentries, + } + } + +-/* Return a read-only mmap of the ulog, or NULL on failure. Assumes fd is +- * released on process exit. */ ++/* Return a read-only mmap of the ulog, or NULL on failure. */ + static kdb_hlog_t * +-map_ulog(const char *filename) ++map_ulog(const char *filename, int *fd_out) + { + int fd; + struct stat st; +- kdb_hlog_t *ulog; ++ kdb_hlog_t *ulog = MAP_FAILED; ++ ++ *fd_out = -1; + + fd = open(filename, O_RDONLY); + if (fd == -1) + return NULL; +- if (fstat(fd, &st) < 0) ++ if (fstat(fd, &st) < 0) { ++ close(fd); + return NULL; ++ } + ulog = mmap(0, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); +- return (ulog == MAP_FAILED) ? NULL : ulog; ++ if (ulog == MAP_FAILED) { ++ close(fd); ++ return NULL; ++ } ++ *fd_out = fd; ++ return ulog; + } + + int + main(int argc, char **argv) + { +- int c; ++ int c, ulog_fd = -1; + unsigned int verbose = 0; + bool_t headeronly = FALSE, reset = FALSE; + uint32_t entry = 0; +@@ -480,7 +488,7 @@ main(int argc, char **argv) + goto done; + } + +- ulog = map_ulog(params.iprop_logfile); ++ ulog = map_ulog(params.iprop_logfile, &ulog_fd); + if (ulog == NULL) { + fprintf(stderr, _("Unable to map log file %s\n\n"), + params.iprop_logfile); +@@ -546,6 +554,7 @@ main(int argc, char **argv) + printf("\n"); + + done: ++ close(ulog_fd); + kadm5_free_config_params(context, ¶ms); + krb5_free_context(context); + return 0; +diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c +index d74dc2b1b..ba7765cb4 100644 +--- a/src/lib/gssapi/spnego/spnego_mech.c ++++ b/src/lib/gssapi/spnego/spnego_mech.c +@@ -3485,11 +3485,9 @@ get_mech_set(OM_uint32 *minor_status, unsigned char **buff_in, + + major_status = gss_add_oid_set_member(minor_status, + temp, &returned_mechSet); +- if (major_status == GSS_S_COMPLETE) { ++ if (major_status == GSS_S_COMPLETE) + set_length += returned_mechSet->elements[i].length +2; +- if (generic_gss_release_oid(minor_status, &temp)) +- map_errcode(minor_status); +- } ++ generic_gss_release_oid(minor_status, &temp); + } + + return (returned_mechSet); +diff --git a/src/lib/kadm5/srv/pwqual_dict.c b/src/lib/kadm5/srv/pwqual_dict.c +index e3ac20eb8..bdd44c23a 100644 +--- a/src/lib/kadm5/srv/pwqual_dict.c ++++ b/src/lib/kadm5/srv/pwqual_dict.c +@@ -121,11 +121,16 @@ init_dict(dict_moddata dict, const char *dict_file) + close(fd); + return errno; + } +- if ((dict->word_block = malloc(sb.st_size + 1)) == NULL) ++ dict->word_block = malloc(sb.st_size + 1); ++ if (dict->word_block == NULL) { ++ (void)close(fd); + return ENOMEM; +- if (read(fd, dict->word_block, sb.st_size) != sb.st_size) ++ } ++ if (read(fd, dict->word_block, sb.st_size) != sb.st_size) { ++ (void)close(fd); + return errno; +- (void) close(fd); ++ } ++ (void)close(fd); + dict->word_block[sb.st_size] = '\0'; + + p = dict->word_block; +diff --git a/src/lib/kdb/kdb5.c b/src/lib/kdb/kdb5.c +index 11e2430c4..c49721821 100644 +--- a/src/lib/kdb/kdb5.c ++++ b/src/lib/kdb/kdb5.c +@@ -1449,8 +1449,11 @@ krb5_db_setup_mkey_name(krb5_context context, const char *keyname, + if (asprintf(&fname, "%s%s%s", keyname, REALM_SEP_STRING, realm) < 0) + return ENOMEM; + +- if ((retval = krb5_parse_name(context, fname, principal))) ++ retval = krb5_parse_name(context, fname, principal); ++ if (retval) { ++ free(fname); + return retval; ++ } + if (fullname) + *fullname = fname; + else +diff --git a/src/lib/kdb/kdb_convert.c b/src/lib/kdb/kdb_convert.c +index e1bf1919f..59952f55e 100644 +--- a/src/lib/kdb/kdb_convert.c ++++ b/src/lib/kdb/kdb_convert.c +@@ -550,7 +550,7 @@ krb5_error_code + ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + kdb_incr_update_t *update) + { +- krb5_db_entry *ent; ++ krb5_db_entry *ent = NULL; + int replica; + krb5_principal mod_princ = NULL; + int i, j, cnt = 0, mod_time = 0, nattrs; +@@ -572,23 +572,20 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + */ + nattrs = update->kdb_update.kdbe_t_len; + +- dbprincstr = malloc((update->kdb_princ_name.utf8str_t_len + 1) +- * sizeof (char)); ++ dbprincstr = k5memdup0(update->kdb_princ_name.utf8str_t_val, ++ update->kdb_princ_name.utf8str_t_len, &ret); + if (dbprincstr == NULL) +- return (ENOMEM); +- strncpy(dbprincstr, (char *)update->kdb_princ_name.utf8str_t_val, +- update->kdb_princ_name.utf8str_t_len); +- dbprincstr[update->kdb_princ_name.utf8str_t_len] = 0; ++ goto cleanup; + + ret = krb5_parse_name(context, dbprincstr, &dbprinc); + free(dbprincstr); + if (ret) +- return (ret); ++ goto cleanup; + + ret = krb5_db_get_principal(context, dbprinc, 0, &ent); + krb5_free_principal(context, dbprinc); + if (ret && ret != KRB5_KDB_NOENTRY) +- return (ret); ++ goto cleanup; + is_add = (ret == KRB5_KDB_NOENTRY); + + /* +@@ -643,8 +640,10 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + + case AT_PRINC: + tmpprinc = conv_princ_2db(context, &u.av_princ); +- if (tmpprinc == NULL) +- return ENOMEM; ++ if (tmpprinc == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + krb5_free_principal(context, ent->princ); + ent->princ = tmpprinc; + break; +@@ -661,8 +660,10 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + /* Allocate one extra key data to avoid allocating zero bytes. */ + newptr = realloc(ent->key_data, (ent->n_key_data + 1) * + sizeof(krb5_key_data)); +- if (newptr == NULL) +- return ENOMEM; ++ if (newptr == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + ent->key_data = newptr; + + /* BEGIN CSTYLED */ +@@ -677,7 +678,8 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + kp->key_data_ver = (krb5_int16)kv->k_ver; + kp->key_data_kvno = (krb5_ui_2)kv->k_kvno; + if (kp->key_data_ver > 2) { +- return EINVAL; /* XXX ? */ ++ ret = EINVAL; /* XXX ? */ ++ goto cleanup; + } + + for (cnt = 0; cnt < kp->key_data_ver; cnt++) { +@@ -685,8 +687,10 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + kp->key_data_length[cnt] = (krb5_int16)kv->k_contents.k_contents_val[cnt].utf8str_t_len; + newptr = realloc(kp->key_data_contents[cnt], + kp->key_data_length[cnt]); +- if (newptr == NULL) +- return ENOMEM; ++ if (newptr == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + kp->key_data_contents[cnt] = newptr; + + (void) memset(kp->key_data_contents[cnt], 0, +@@ -704,22 +708,26 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + newtl.tl_data_length = (krb5_int16)u.av_tldata.av_tldata_val[j].tl_data.tl_data_len; + newtl.tl_data_contents = (krb5_octet *)u.av_tldata.av_tldata_val[j].tl_data.tl_data_val; + newtl.tl_data_next = NULL; +- if ((ret = krb5_dbe_update_tl_data(context, ent, &newtl))) +- return (ret); ++ ret = krb5_dbe_update_tl_data(context, ent, &newtl); ++ if (ret) ++ goto cleanup; + } + break; + /* END CSTYLED */ + } + case AT_PW_LAST_CHANGE: +- if ((ret = krb5_dbe_update_last_pwd_change(context, ent, +- u.av_pw_last_change))) +- return (ret); ++ ret = krb5_dbe_update_last_pwd_change(context, ent, ++ u.av_pw_last_change); ++ if (ret) ++ goto cleanup; + break; + + case AT_MOD_PRINC: + tmpprinc = conv_princ_2db(context, &u.av_mod_princ); +- if (tmpprinc == NULL) +- return ENOMEM; ++ if (tmpprinc == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + mod_princ = tmpprinc; + break; + +@@ -743,14 +751,17 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + if (mod_time && mod_princ) { + ret = krb5_dbe_update_mod_princ_data(context, ent, + mod_time, mod_princ); +- krb5_free_principal(context, mod_princ); +- mod_princ = NULL; + if (ret) +- return (ret); ++ goto cleanup; + } + + *entry = ent; +- return (0); ++ ent = NULL; ++ ret = 0; ++cleanup: ++ krb5_db_free_principal(context, ent); ++ krb5_free_principal(context, mod_princ); ++ return ret; + } + + +diff --git a/src/lib/krad/remote.c b/src/lib/krad/remote.c +index a938665f6..7e491e994 100644 +--- a/src/lib/krad/remote.c ++++ b/src/lib/krad/remote.c +@@ -445,7 +445,7 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs, + { + krad_packet *tmp = NULL; + krb5_error_code retval; +- request *r; ++ request *r, *new_request = NULL; + + if (rr->info->ai_socktype == SOCK_STREAM) + retries = 0; +@@ -464,7 +464,7 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs, + } + + timeout = timeout / (retries + 1); +- retval = request_new(rr, tmp, timeout, retries, cb, data, &r); ++ retval = request_new(rr, tmp, timeout, retries, cb, data, &new_request); + if (retval != 0) + goto error; + +@@ -472,12 +472,13 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs, + if (retval != 0) + goto error; + +- K5_TAILQ_INSERT_TAIL(&rr->list, r, list); ++ K5_TAILQ_INSERT_TAIL(&rr->list, new_request, list); + if (pkt != NULL) + *pkt = tmp; + return 0; + + error: ++ free(new_request); + krad_packet_free(tmp); + return retval; + } +diff --git a/src/lib/krb5/krb/gic_keytab.c b/src/lib/krb5/krb/gic_keytab.c +index d20457069..b8b7c1506 100644 +--- a/src/lib/krb5/krb/gic_keytab.c ++++ b/src/lib/krb5/krb/gic_keytab.c +@@ -182,7 +182,7 @@ krb5_init_creds_set_keytab(krb5_context context, + krb5_init_creds_context ctx, + krb5_keytab keytab) + { +- krb5_enctype *etype_list; ++ krb5_enctype *etype_list = NULL; + krb5_error_code ret; + struct canonprinc iter = { ctx->request->client, .subst_defrealm = TRUE }; + krb5_const_principal canonprinc; +@@ -212,6 +212,7 @@ krb5_init_creds_set_keytab(krb5_context context, + free_canonprinc(&iter); + if (ret) { + TRACE_INIT_CREDS_KEYTAB_LOOKUP_FAILED(context, ret); ++ free(etype_list); + return 0; + } + TRACE_INIT_CREDS_KEYTAB_LOOKUP(context, ctx->request->client, etype_list); +diff --git a/src/lib/krb5/krb/s4u_authdata.c b/src/lib/krb5/krb/s4u_authdata.c +index a2300def8..c33a50a56 100644 +--- a/src/lib/krb5/krb/s4u_authdata.c ++++ b/src/lib/krb5/krb/s4u_authdata.c +@@ -170,8 +170,10 @@ s4u2proxy_export_authdata(krb5_context kcontext, + return code; + + authdata[0] = k5alloc(sizeof(krb5_authdata), &code); +- if (authdata[0] == NULL) ++ if (authdata[0] == NULL) { ++ free(authdata); + return code; ++ } + + code = encode_krb5_ad_signedpath(&sp, &data); + if (code != 0) { +diff --git a/src/lib/krb5/krb/send_tgs.c b/src/lib/krb5/krb/send_tgs.c +index 3dda2fdaa..7a11864fa 100644 +--- a/src/lib/krb5/krb/send_tgs.c ++++ b/src/lib/krb5/krb/send_tgs.c +@@ -261,8 +261,10 @@ k5_make_tgs_req(krb5_context context, + pa->length = in_padata[i]->length; + pa->contents = k5memdup(in_padata[i]->contents, in_padata[i]->length, + &ret); +- if (pa->contents == NULL) ++ if (pa->contents == NULL) { ++ free(pa); + goto cleanup; ++ } + padata[i + 1] = pa; + } + req.padata = padata; +@@ -289,7 +291,7 @@ cleanup: + krb5_free_data(context, authdata_asn1); + krb5_free_data(context, req_body_asn1); + krb5_free_data(context, ap_req_asn1); +- krb5_free_pa_data(context, req.padata); ++ krb5_free_pa_data(context, padata); + krb5_free_ticket(context, sec_ticket); + krb5_free_data_contents(context, &authdata_enc.ciphertext); + krb5_free_keyblock(context, subkey); +diff --git a/src/lib/krb5/os/dnssrv.c b/src/lib/krb5/os/dnssrv.c +index 02ba87955..5992a9bdf 100644 +--- a/src/lib/krb5/os/dnssrv.c ++++ b/src/lib/krb5/os/dnssrv.c +@@ -217,7 +217,7 @@ k5_make_uri_query(krb5_context context, const krb5_data *realm, + /* rdlen - 4 bytes remain after the priority and weight. */ + uri->host = k5memdup0(p, rdlen - 4, &ret); + if (uri->host == NULL) { +- ret = errno; ++ free(uri); + goto out; + } + +diff --git a/src/lib/krb5/os/sendto_kdc.c b/src/lib/krb5/os/sendto_kdc.c +index 0eedec175..8620ffa45 100644 +--- a/src/lib/krb5/os/sendto_kdc.c ++++ b/src/lib/krb5/os/sendto_kdc.c +@@ -722,8 +722,10 @@ add_connection(struct conn_state **conns, k5_transport transport, + + if (*udpbufp == NULL) { + *udpbufp = malloc(MAX_DGRAM_SIZE); +- if (*udpbufp == 0) ++ if (*udpbufp == NULL) { ++ free(state); + return ENOMEM; ++ } + } + state->in.buf = *udpbufp; + state->in.bufsize = MAX_DGRAM_SIZE; +diff --git a/src/plugins/audit/kdc_j_encode.c b/src/plugins/audit/kdc_j_encode.c +index 265e95bc4..fb4a4ed73 100755 +--- a/src/plugins/audit/kdc_j_encode.c ++++ b/src/plugins/audit/kdc_j_encode.c +@@ -748,9 +748,8 @@ req_to_value(krb5_kdc_req *req, const krb5_boolean ev_success, + if (ret) + goto error; + ret = addr_to_obj(req->addresses[i], tmpa); +- if (ret) +- goto error; +- ret = k5_json_array_add(arra, tmpa); ++ if (!ret) ++ ret = k5_json_array_add(arra, tmpa); + k5_json_release(tmpa); + if (ret) + goto error; +diff --git a/src/plugins/kdb/db2/adb_policy.c b/src/plugins/kdb/db2/adb_policy.c +index 9bf1931da..221473bb0 100644 +--- a/src/plugins/kdb/db2/adb_policy.c ++++ b/src/plugins/kdb/db2/adb_policy.c +@@ -337,16 +337,16 @@ osa_adb_iter_policy(osa_adb_policy_t db, osa_adb_iter_policy_func func, + } + + while (ret == 0) { +- if (!(entry = (osa_policy_ent_t) malloc(sizeof(osa_policy_ent_rec)))) { +- ret = ENOMEM; ++ entry = k5alloc(sizeof(osa_policy_ent_rec), &ret); ++ if (entry == NULL) + goto error; +- } + + aligned_data = k5memdup(dbdata.data, dbdata.size, &ret); +- if (aligned_data == NULL) ++ if (aligned_data == NULL) { ++ free(entry); + goto error; ++ } + +- memset(entry, 0, sizeof(osa_policy_ent_rec)); + xdrmem_create(&xdrs, aligned_data, dbdata.size, XDR_DECODE); + if(!xdr_osa_policy_ent_rec(&xdrs, entry)) { + xdr_destroy(&xdrs); +diff --git a/src/plugins/kdb/db2/kdb_db2.c b/src/plugins/kdb/db2/kdb_db2.c +index 1a476b586..2c163d91c 100644 +--- a/src/plugins/kdb/db2/kdb_db2.c ++++ b/src/plugins/kdb/db2/kdb_db2.c +@@ -1258,7 +1258,7 @@ krb5_db2_destroy(krb5_context context, char *conf_section, char **db_args) + goto cleanup; + status = osa_adb_destroy_db(polname, plockname, OSA_ADB_POLICY_DB_MAGIC); + if (status) +- return status; ++ goto cleanup; + + status = krb5_db2_fini(context); + +diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c +index ce28bf61f..b5a4e5f76 100644 +--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c ++++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c +@@ -188,20 +188,22 @@ krb5_ldap_iterate(krb5_context context, char *match_expr, + for (i=0; values[i] != NULL; ++i) { + if (krb5_ldap_parse_principal_name(values[i], &princ_name) != 0) + continue; +- if (krb5_parse_name(context, princ_name, &principal) != 0) ++ st = krb5_parse_name(context, princ_name, &principal); ++ free(princ_name); ++ if (st) + continue; ++ + if (is_principal_in_realm(ldap_context, principal)) { +- if ((st = populate_krb5_db_entry(context, ldap_context, ld, ent, principal, +- &entry)) != 0) ++ st = populate_krb5_db_entry(context, ldap_context, ld, ++ ent, principal, &entry); ++ krb5_free_principal(context, principal); ++ if (st) + goto cleanup; + (*func)(func_arg, &entry); + krb5_dbe_free_contents(context, &entry); +- (void) krb5_free_principal(context, principal); +- free(princ_name); + break; + } + (void) krb5_free_principal(context, principal); +- free(princ_name); + } + ldap_value_free(values); + } +diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +index 8d97a29b6..ff705a2cc 100644 +--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c ++++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +@@ -785,6 +785,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + char *polname = NULL; + OPERATION optype; + krb5_boolean found_entry = FALSE; ++ char *filter = NULL; + + /* Clear the global error string */ + krb5_clear_error_message(context); +@@ -833,7 +834,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + if (entry->mask & KADM5_LOAD) { + unsigned int tree = 0; + int numlentries = 0; +- char *filter = NULL; + + /* A load operation is special, will do a mix-in (add krbprinc + * attrs to a non-krb object entry) if an object exists with a +@@ -864,7 +864,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + if (st == LDAP_SUCCESS) { + numlentries = ldap_count_entries(ld, result); + if (numlentries > 1) { +- free(filter); + st = EINVAL; + k5_setmsg(context, st, + _("operation can not continue, more than one " +@@ -880,7 +879,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + if ((principal_dn = ldap_get_dn(ld, ent)) == NULL) { + ldap_get_option (ld, LDAP_OPT_RESULT_CODE, &st); + st = set_ldap_error (context, st, 0); +- free(filter); + goto cleanup; + } + } +@@ -889,7 +887,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + } else if (st != LDAP_NO_SUCH_OBJECT) { + /* could not perform search, return with failure */ + st = set_ldap_error (context, st, 0); +- free(filter); + goto cleanup; + } + ldap_msgfree(result); +@@ -900,8 +897,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + */ + } /* end for (tree = 0; principal_dn == ... */ + +- free(filter); +- + if (found_entry == FALSE && principal_dn != NULL) { + /* + * if principal_dn is null then there is code further down to +@@ -1450,6 +1445,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + } + + cleanup: ++ free(filter); + if (user) + free(user); + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index ce4233953..0204ad8ba 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -3653,14 +3653,15 @@ static krb5_error_code + pkinit_open_session(krb5_context context, + pkinit_identity_crypto_context cctx) + { +- CK_ULONG i, r; ++ CK_ULONG i, pret; + unsigned char *cp; + size_t label_len; + CK_ULONG count = 0; +- CK_SLOT_ID_PTR slotlist; ++ CK_SLOT_ID_PTR slotlist = NULL; + CK_TOKEN_INFO tinfo; +- char *p11name; ++ char *p11name = NULL; + const char *password; ++ krb5_error_code ret; + + if (cctx->p11_module != NULL) + return 0; /* session already open */ +@@ -3672,8 +3673,9 @@ pkinit_open_session(krb5_context context, + return KRB5KDC_ERR_PREAUTH_FAILED; + + /* Init */ +- if ((r = cctx->p11->C_Initialize(NULL)) != CKR_OK) { +- pkiDebug("C_Initialize: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pret = cctx->p11->C_Initialize(NULL); ++ if (pret != CKR_OK) { ++ pkiDebug("C_Initialize: %s\n", pkinit_pkcs11_code_to_text(pret)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +@@ -3687,8 +3689,10 @@ pkinit_open_session(krb5_context context, + slotlist = calloc(count, sizeof(CK_SLOT_ID)); + if (slotlist == NULL) + return ENOMEM; +- if (cctx->p11->C_GetSlotList(TRUE, slotlist, &count) != CKR_OK) +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ if (cctx->p11->C_GetSlotList(TRUE, slotlist, &count) != CKR_OK) { ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; ++ } + + /* Look for the given token label, or if none given take the first one */ + for (i = 0; i < count; i++) { +@@ -3697,16 +3701,20 @@ pkinit_open_session(krb5_context context, + continue; + + /* Open session */ +- if ((r = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION, +- NULL, NULL, &cctx->session)) != CKR_OK) { +- pkiDebug("C_OpenSession: %s\n", pkinit_pkcs11_code_to_text(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ pret = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION, ++ NULL, NULL, &cctx->session); ++ if (pret != CKR_OK) { ++ pkiDebug("C_OpenSession: %s\n", pkinit_pkcs11_code_to_text(pret)); ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; + } + + /* Get token info */ +- if ((r = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo)) != CKR_OK) { +- pkiDebug("C_GetTokenInfo: %s\n", pkinit_pkcs11_code_to_text(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ pret = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo); ++ if (pret != CKR_OK) { ++ pkiDebug("C_GetTokenInfo: %s\n", pkinit_pkcs11_code_to_text(pret)); ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; + } + + /* tinfo.label is zero-filled but not necessarily zero-terminated. +@@ -3726,12 +3734,11 @@ pkinit_open_session(krb5_context context, + cctx->p11->C_CloseSession(cctx->session); + } + if (i >= count) { +- free(slotlist); + pkiDebug("open_session: no matching token found\n"); +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; + } + cctx->slotid = slotlist[i]; +- free(slotlist); + pkiDebug("open_session: slotid %d (%lu of %d)\n", (int)cctx->slotid, + i + 1, (int) count); + +@@ -3751,23 +3758,26 @@ pkinit_open_session(krb5_context context, + (int)label_len, tinfo.label) < 0) + p11name = NULL; + } +- } else { +- p11name = NULL; + } + if (cctx->defer_id_prompt) { + /* Supply the identity name to be passed to the responder. */ + pkinit_set_deferred_id(&cctx->deferred_ids, + p11name, tinfo.flags, NULL); +- free(p11name); +- return 0; ++ ret = 0; ++ goto cleanup; + } + /* Look up a responder-supplied password for the token. */ + password = pkinit_find_deferred_id(cctx->deferred_ids, p11name); +- free(p11name); +- r = pkinit_login(context, cctx, &tinfo, password); ++ ret = pkinit_login(context, cctx, &tinfo, password); ++ if (ret) ++ goto cleanup; + } + +- return r; ++ ret = 0; ++cleanup: ++ free(slotlist); ++ free(p11name); ++ return ret; + } + + /* +diff --git a/src/plugins/preauth/pkinit/pkinit_identity.c b/src/plugins/preauth/pkinit/pkinit_identity.c +index 3c1778a3b..a5a979f27 100644 +--- a/src/plugins/preauth/pkinit/pkinit_identity.c ++++ b/src/plugins/preauth/pkinit/pkinit_identity.c +@@ -310,17 +310,17 @@ parse_fs_options(krb5_context context, + const char *residual) + { + char *certname, *keyname, *save; +- char *cert_filename = NULL, *key_filename = NULL; ++ char *copy = NULL, *cert_filename = NULL, *key_filename = NULL; + krb5_error_code retval = ENOMEM; + + if (residual == NULL || residual[0] == '\0' || residual[0] == ',') + return EINVAL; + +- certname = strdup(residual); +- if (certname == NULL) ++ copy = strdup(residual); ++ if (copy == NULL) + goto cleanup; + +- certname = strtok_r(certname, ",", &save); ++ certname = strtok_r(copy, ",", &save); + if (certname == NULL) + goto cleanup; + keyname = strtok_r(NULL, ",", &save); +@@ -341,7 +341,7 @@ parse_fs_options(krb5_context context, + retval = 0; + + cleanup: +- free(certname); ++ free(copy); + free(cert_filename); + free(key_filename); + return retval; +diff --git a/src/util/profile/prof_get.c b/src/util/profile/prof_get.c +index 16a1762df..0e14200ca 100644 +--- a/src/util/profile/prof_get.c ++++ b/src/util/profile/prof_get.c +@@ -157,7 +157,7 @@ profile_get_values(profile_t profile, const char *const *names, + char ***ret_values) + { + errcode_t retval; +- void *state; ++ void *state = NULL; + char *value; + struct profile_string_list values; + +@@ -172,8 +172,9 @@ profile_get_values(profile_t profile, const char *const *names, + &state))) + return retval; + +- if ((retval = init_list(&values))) +- return retval; ++ retval = init_list(&values); ++ if (retval) ++ goto cleanup; + + do { + if ((retval = profile_node_iterator(&state, 0, 0, &value))) +@@ -187,11 +188,9 @@ profile_get_values(profile_t profile, const char *const *names, + goto cleanup; + } + +- end_list(&values, ret_values); +- return 0; +- + cleanup: +- end_list(&values, 0); ++ end_list(&values, retval ? NULL : ret_values); ++ profile_node_iterator_free(&state); + return retval; + } + +-- +2.25.1 + diff --git a/Free-verto-context-later-in-KDC-cleanup.patch b/Free-verto-context-later-in-KDC-cleanup.patch new file mode 100644 index 0000000000000000000000000000000000000000..192d29dc58c18c89d51e25f8382f3d5b72a0c41c --- /dev/null +++ b/Free-verto-context-later-in-KDC-cleanup.patch @@ -0,0 +1,44 @@ +From 8dcace04945723cd6a3c8ea2c1ba467c22eb6584 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Fri, 3 Jun 2022 14:38:45 -0400 +Subject: [PATCH] Free verto context later in KDC cleanup + +The KDC supplies the verto context to kdcpreauth modules via the loop +method (added in commit 83b4ecd20e50ad330cd761977d5dadefe30a785b). +This context should remain valid until kdcpreauth modules are +unloaded, as modules might refer to it during cleanup. In particular, +the OTP module references the verto context when freeing the RADIUS +client object (commit e89abc2d4ea1fea1ec28d470f297514b828e4842), which +can cause a memory error during KDC shutdown without this change. + +ticket: 9064 (new) +tags: pullup +target_version: 1.20-next +target_version: 1.19-next +--- + src/kdc/main.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/kdc/main.c b/src/kdc/main.c +index be6e361b8..bfdfef5c4 100644 +--- a/src/kdc/main.c ++++ b/src/kdc/main.c +@@ -1037,7 +1037,6 @@ int main(int argc, char **argv) + kau_kdc_start(kcontext, TRUE); + + verto_run(ctx); +- loop_free(ctx); + kau_kdc_stop(kcontext, TRUE); + krb5_klog_syslog(LOG_INFO, _("shutting down")); + unload_preauth_plugins(kcontext); +@@ -1051,6 +1050,7 @@ int main(int argc, char **argv) + #ifndef NOCACHE + kdc_free_lookaside(kcontext); + #endif ++ loop_free(ctx); + krb5_free_context(kcontext); + return errout; + } +-- +2.25.1 + diff --git a/krb5.spec b/krb5.spec index c8c80e9cb8acca32e22535066b68d1f23599ed8b..ae098aa297a33591cc62b1579252808fbe628fa2 100644 --- a/krb5.spec +++ b/krb5.spec @@ -3,7 +3,7 @@ Name: krb5 Version: 1.19.2 -Release: 3 +Release: 4 Summary: The Kerberos network authentication protocol License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -27,6 +27,8 @@ Patch4: fix-debuginfo-with-y.tab.c.patch Patch5: Remove-3des-support.patch Patch6: FIPS-with-PRNG-and-RADIUS-and-MD4.patch Patch7: backport-CVE-2021-37750.patch +Patch8: Fix-many-unlikely-memory-leaks.patch +Patch9: Free-verto-context-later-in-KDC-cleanup.patch BuildRequires: gettext BuildRequires: gcc make automake autoconf pkgconfig pam-devel libselinux-devel byacc @@ -318,6 +320,13 @@ make -C src check || : %{_mandir}/man8/* %changelog +* Wed Oct 26 2022 zhangjun - 1.19.2-4 +- Type:bugfix +- ID:NA +- SUG:NA +- DESC:Fix many unlikely memory leaks + Free verto context later in KDC cleanup + * Wed Oct 19 2022 zhouchenchen - 1.19.2-3 - update version to 1.19.2-3