krb5 commit: Fix many unlikely memory leaks
Greg Hudson <[email protected]>
| Newsgroups | gmane.comp.encryption.kerberos.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://github.com/krb5/krb5/commit/a06945b4ec267e8b80e5e8c95edd89930ff12103 commit a06945b4ec267e8b80e5e8c95edd89930ff12103 Author: Robbie Harwood <[email protected]> Date: Wed May 26 17:35:06 2021 -0400 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 +- src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c | 14 +++-- src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c | 8 +-- src/plugins/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 42b45aa..db12cac 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 efdb8ad..f9205f8 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 e2e005d..56bed1b 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 6d244ff..582e497 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 498ca59..46bb9d1 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 0c025f0..9d3a910 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 d74dc2b..ba7765c 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 e3ac20e..bdd44c2 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 11e2430..c497218 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 e1bf191..59952f5 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 a938665..7e491e9 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 d204570..b8b7c15 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 a2300de..c33a50a 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 3dda2fd..7a11864 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 02ba879..5992a9b 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 0eedec1..8620ffa 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 265e95b..fb4a4ed 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 9bf1931..221473b 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 1a476b5..2c163d9 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 ce28bf6..b5a4e5f 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 8d97a29..ff705a2 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 ce42339..0204ad8 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", pkcs11err(r)); + pret = cctx->p11->C_Initialize(NULL); + if (pret != CKR_OK) { + pkiDebug("C_Initialize: %s\n", pkcs11err(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", pkcs11err(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", pkcs11err(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", pkcs11err(r)); - return KRB5KDC_ERR_PREAUTH_FAILED; + pret = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo); + if (pret != CKR_OK) { + pkiDebug("C_GetTokenInfo: %s\n", pkcs11err(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); TRACE_PKINIT_PKCS11_NO_MATCH_TOKEN(context); - 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 3c1778a..a5a979f 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 16a1762..0e14200 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; }