krb5 commit: Fix multiple leaks in ktutil addent
Greg Hudson <[email protected]>
| Newsgroups | gmane.comp.encryption.kerberos.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://github.com/krb5/krb5/commit/76053d61fecfec8c5ea31c74bec73d2846b5effe commit 76053d61fecfec8c5ea31c74bec73d2846b5effe Author: Greg Hudson <[email protected]> Date: Thu Oct 11 15:33:35 2018 -0400 Fix multiple leaks in ktutil addent In ktutil_add(), free allocations on success as well as failure. Change all early returns to jumps to the cleanup label. Free the password buffer and unparsed principal name. Do list manipulation as the final step to simplify cleanup. Reported by Bean Zhang. ticket: 8750 src/kadmin/ktutil/ktutil_funcs.c | 96 +++++++++++++++---------------------- 1 files changed, 39 insertions(+), 57 deletions(-) diff --git a/src/kadmin/ktutil/ktutil_funcs.c b/src/kadmin/ktutil/ktutil_funcs.c index 2daf814..6d119a2 100644 --- a/src/kadmin/ktutil/ktutil_funcs.c +++ b/src/kadmin/ktutil/ktutil_funcs.c @@ -149,73 +149,52 @@ krb5_error_code ktutil_add(context, list, princ_str, fetch, kvno, int use_pass; char *salt_str; { - krb5_keytab_entry *entry; - krb5_kt_list lp = NULL, prev = NULL; + krb5_keytab_entry *entry = NULL; + krb5_kt_list lp, *last; krb5_principal princ; krb5_enctype enctype = ENCTYPE_NULL; krb5_timestamp now; krb5_error_code retval; - krb5_data password, salt = empty_data(), params = empty_data(), *s2kparams; + krb5_data password = empty_data(), salt = empty_data(); + krb5_data params = empty_data(), *s2kparams; krb5_keyblock key; char buf[BUFSIZ]; char promptstr[1024]; + char *princ_full = NULL; uint8_t *keybytes; size_t keylen; unsigned int pwsize = BUFSIZ; retval = krb5_parse_name(context, princ_str, &princ); if (retval) - return retval; + goto cleanup; /* now unparse in order to get the default realm appended to princ_str, if no realm was specified */ - retval = krb5_unparse_name(context, princ, &princ_str); + retval = krb5_unparse_name(context, princ, &princ_full); if (retval) - return retval; + goto cleanup; if (enctype_str != NULL) { retval = krb5_string_to_enctype(enctype_str, &enctype); - if (retval) - return KRB5_BAD_ENCTYPE; + if (retval) { + retval = KRB5_BAD_ENCTYPE; + goto cleanup; + } } retval = krb5_timeofday(context, &now); if (retval) - return retval; + goto cleanup; - if (*list) { - /* point lp at the tail of the list */ - for (lp = *list; lp->next; lp = lp->next); - } - entry = (krb5_keytab_entry *) malloc(sizeof(krb5_keytab_entry)); - if (!entry) { - return ENOMEM; - } - memset(entry, 0, sizeof(*entry)); - - if (!lp) { /* if list is empty, start one */ - lp = (krb5_kt_list) malloc(sizeof(*lp)); - if (!lp) { - return ENOMEM; - } - } else { - lp->next = (krb5_kt_list) malloc(sizeof(*lp)); - if (!lp->next) { - return ENOMEM; - } - prev = lp; - lp = lp->next; - } - lp->next = NULL; - lp->entry = entry; + entry = k5alloc(sizeof(*entry), &retval); + if (entry == NULL) + goto cleanup; if (use_pass) { - password.length = pwsize; - password.data = (char *) malloc(pwsize); - if (!password.data) { - retval = ENOMEM; + retval = alloc_data(&password, pwsize); + if (retval) goto cleanup; - } snprintf(promptstr, sizeof(promptstr), _("Password for %.1000s"), - princ_str); + princ_full); retval = krb5_read_password(context, promptstr, NULL, password.data, &password.length); if (retval) @@ -230,11 +209,9 @@ krb5_error_code ktutil_add(context, list, princ_str, fetch, kvno, &salt, s2kparams, &key); if (retval) goto cleanup; - memset(password.data, 0, password.length); - password.length = 0; - lp->entry->key = key; + entry->key = key; } else { - printf(_("Key for %s (hex): "), princ_str); + printf(_("Key for %s (hex): "), princ_full); fgets(buf, BUFSIZ, stdin); /* * We need to get rid of the trailing '\n' from fgets. @@ -260,25 +237,30 @@ krb5_error_code ktutil_add(context, list, princ_str, fetch, kvno, goto cleanup; } - lp->entry->key.enctype = enctype; - lp->entry->key.contents = keybytes; - lp->entry->key.length = keylen; + entry->key.enctype = enctype; + entry->key.contents = keybytes; + entry->key.length = keylen; } - lp->entry->principal = princ; - lp->entry->vno = kvno; - lp->entry->timestamp = now; - - if (!*list) - *list = lp; + entry->principal = princ; + entry->vno = kvno; + entry->timestamp = now; - return 0; + /* Add entry to the end of the list (or create a new list if empty). */ + lp = k5alloc(sizeof(*lp), &retval); + if (lp == NULL) + goto cleanup; + lp->next = NULL; + lp->entry = entry; + entry = NULL; + for (last = list; *last != NULL; last = &(*last)->next); + *last = lp; cleanup: - if (prev) - prev->next = NULL; - ktutil_free_kt_list(context, lp); + krb5_kt_free_entry(context, entry); + zapfree(password.data, password.length); krb5_free_data_contents(context, &salt); krb5_free_data_contents(context, ¶ms); + krb5_free_unparsed_name(context, princ_full); return retval; }