krb5 commit: Fix many unlikely memory leaks
Greg Hudson
ghudson at mit.edu
Thu Jul 1 12:11:44 EDT 2021
https://github.com/krb5/krb5/commit/a06945b4ec267e8b80e5e8c95edd89930ff12103
commit a06945b4ec267e8b80e5e8c95edd89930ff12103
Author: Robbie Harwood <rharwood at redhat.com>
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;
}
More information about the cvs-krb5
mailing list