From 817907e0332074574abf2ef7ff27ad9bd6682571 Mon Sep 17 00:00:00 2001 From: Theodore Tso Date: Sat, 25 Jun 1994 03:19:10 +0000 Subject: [PATCH] Checked in rest of Jim Miller's kadmin bugs krb5-bugs [0395] git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@3915 dc483132-0cff-0310-8789-dd5450dbe970 --- src/kadmin/client/ChangeLog | 5 ++ src/kadmin/client/kadmin_cpw.c | 5 ++ src/kadmin/client/kadmin_del.c | 1 + src/kadmin/client/kadmin_done.c | 11 ++-- src/kadmin/client/kadmin_mod.c | 4 +- src/kadmin/server/ChangeLog | 11 ++++ src/kadmin/server/adm_fmt_inq.c | 28 +++++---- src/kadmin/server/adm_funcs.c | 105 ++++++++++++++++++++------------ src/kadmin/server/adm_kadmin.c | 6 +- src/kadmin/server/adm_nego.c | 1 + src/kadmin/server/adm_process.c | 8 +-- 11 files changed, 118 insertions(+), 67 deletions(-) create mode 100644 src/kadmin/client/ChangeLog diff --git a/src/kadmin/client/ChangeLog b/src/kadmin/client/ChangeLog new file mode 100644 index 000000000..56454bad2 --- /dev/null +++ b/src/kadmin/client/ChangeLog @@ -0,0 +1,5 @@ +Fri Jun 24 22:48:29 1994 Theodore Y. Ts'o (tytso at tsx-11) + + * kadmin_done (kadm_done): fix memory allocation bugs + + diff --git a/src/kadmin/client/kadmin_cpw.c b/src/kadmin/client/kadmin_cpw.c index fc67d2189..f12480edc 100644 --- a/src/kadmin/client/kadmin_cpw.c +++ b/src/kadmin/client/kadmin_cpw.c @@ -119,6 +119,7 @@ char *principal; /* write private message to server */ if (krb5_write_message(local_socket, &msg_data)){ + free(msg_data.data); fprintf(stderr, "Write Error During Second Message Transmission!\n"); return(1); } @@ -154,6 +155,8 @@ char *principal; free(rd_priv_resp.message); } else fprintf(stderr, "Generic error from server.\n\n"); + memset(msg_data.data, 0, msg_data.length); + free(msg_data.data); return(0); } @@ -167,6 +170,8 @@ char *principal; pwsize = msg_data.length; if ((password = (char *) calloc (1, pwsize)) == (char *) 0) { fprintf(stderr, "No Memory for allocation of password!\n"); + memset(msg_data.data, 0, msg_data.length); + free(msg_data.data); return(1); } diff --git a/src/kadmin/client/kadmin_del.c b/src/kadmin/client/kadmin_del.c index 8a021d53a..a2d83673d 100644 --- a/src/kadmin/client/kadmin_del.c +++ b/src/kadmin/client/kadmin_del.c @@ -112,6 +112,7 @@ char *principal; /* write private message to server */ if (krb5_write_message(local_socket, &msg_data)){ + free(msg_data.data); fprintf(stderr, "Write Error During Second Message Transmission!\n"); return(1); } diff --git a/src/kadmin/client/kadmin_done.c b/src/kadmin/client/kadmin_done.c index 08ad01812..8655f5dce 100644 --- a/src/kadmin/client/kadmin_done.c +++ b/src/kadmin/client/kadmin_done.c @@ -51,12 +51,9 @@ krb5_int32 *seqno; { krb5_data msg_data, inbuf; krb5_error_code retval; /* return code */ + char buf[16]; - /* XXX 755 was sizeof( char username[755]) */ - if ((inbuf.data = (char *) calloc(1, 8 + 755)) == (char *) 0) { - fprintf(stderr, "No memory for command!\n"); - exit(1); - } + inbuf.data = buf; inbuf.data[0] = KADMIN; inbuf.data[1] = COMPLETE; @@ -79,10 +76,10 @@ krb5_int32 *seqno; error_message(retval)); return(1); } - free(inbuf.data); /* write private message to server */ - if (krb5_write_message(local_socket, &msg_data)){ + if (krb5_write_message(local_socket, &msg_data)) { + free(msg_data.data); fprintf(stderr, "Write Error During Second Message Transmission!\n"); return(1); } diff --git a/src/kadmin/client/kadmin_mod.c b/src/kadmin/client/kadmin_mod.c index 7e773d793..0d895b0bb 100644 --- a/src/kadmin/client/kadmin_mod.c +++ b/src/kadmin/client/kadmin_mod.c @@ -139,7 +139,6 @@ char *principal; return(1); } free(inbuf.data); - free(msg_data.data); if (msg_data.data[2] == KADMBAD) { decode_kadmind_reply(msg_data, &rd_priv_resp); @@ -149,8 +148,10 @@ char *principal; free(rd_priv_resp.message); } else fprintf(stderr, "Generic error from server.\n\n"); + free(msg_data.data); return(0); } + free(msg_data.data); kadm_snd_mod(my_creds, rep_ret, local_addr, foreign_addr, local_socket, seqno); @@ -186,6 +187,7 @@ char *principal; /* write private message to server */ if (krb5_write_message(local_socket, &msg_data)){ fprintf(stderr, "Write Error During Second Message Transmission!\n"); + free(msg_data.data); return(1); } free(msg_data.data); diff --git a/src/kadmin/server/ChangeLog b/src/kadmin/server/ChangeLog index 50600c6b3..e71cdd6ba 100644 --- a/src/kadmin/server/ChangeLog +++ b/src/kadmin/server/ChangeLog @@ -1,5 +1,16 @@ Fri Jun 24 20:39:37 1994 Theodore Y. Ts'o (tytso at tsx-11) + * adm_process.c (cpw_keyproc): return error codes on failure + + * adm_nego.c (adm_negotiate_key): added return on memory + allocation error + + * adm_fmt_inq.c (adm_fmt_prt, adm_print_exp_time, + adm_print_attributes): Sanitized error return strategies. + + * adm_kadmin.c (adm5_kadmin): Plug memory leaks, fix double + free's, fix message in error syslog. + * adm_process.c (process_client): Plug memory leaks * adm_adm_func. (adm_inq_old_key): Plug memory leaks, return error diff --git a/src/kadmin/server/adm_fmt_inq.c b/src/kadmin/server/adm_fmt_inq.c index a8c561e90..698203772 100644 --- a/src/kadmin/server/adm_fmt_inq.c +++ b/src/kadmin/server/adm_fmt_inq.c @@ -39,7 +39,6 @@ static char rcsid_adm_fmt_inq[] = #include #include -#include #include #ifdef USE_SYS_TIME_H @@ -61,9 +60,8 @@ krb5_flags attribs; { char *my_data; - if ((my_data = (char *) calloc (1,255)) == (char *) 0) { - com_err("adm_print_attributes", ENOMEM, ""); - } + if ((my_data = (char *) calloc (1,255)) == (char *) 0) + return ENOMEM; sprintf(my_data, "Principal Attributes (PA): "); if (attribs & KRB5_KDB_DISALLOW_POSTDATED) @@ -128,9 +126,8 @@ krb5_timestamp *time_input; char *my_data; struct tm *exp_time; - if ((my_data = (char *) calloc (1,255)) == (char *) 0) { - com_err("adm_print_attributes", ENOMEM, ""); - } + if ((my_data = (char *) calloc (1,255)) == (char *) 0) + return ENOMEM; exp_time = localtime((time_t *) time_input); sprintf(my_data, @@ -154,8 +151,8 @@ char *Principal_name; char *ret_data; { struct tm *mod_time; -#ifdef SANDIA krb5_error_code retval; +#ifdef SANDIA struct tm *exp_time; int pwd_expire; krb5_timestamp now; @@ -164,9 +161,8 @@ char *ret_data; char *my_data; char thisline[80]; - if ((my_data = (char *) calloc (1, 2048)) == (char *) 0) { - com_err("adm_print_attributes", ENOMEM, ""); - } + if ((my_data = (char *) calloc (1, 2048)) == (char *) 0) + return ENOMEM; (void) sprintf(my_data, "\n\nPrincipal: %s\n\n", Principal_name); sprintf(thisline, @@ -177,7 +173,10 @@ char *ret_data; strcat(my_data, thisline); sprintf(thisline, "Principal Key Version (PKV) = %d\n", entry->kvno); strcat(my_data, thisline); - (void) adm_print_exp_time(my_data, &entry->expiration); + if (retval = adm_print_exp_time(my_data, &entry->expiration)) { + free(my_data); + return retval; + } mod_time = localtime((time_t *) &entry->mod_date); sprintf(thisline, "Last Modification Date (LMD): %02d%02d/%02d/%02d:%02d:%02d:%02d\n", @@ -189,7 +188,10 @@ char *ret_data; mod_time->tm_min, mod_time->tm_sec); strcat(my_data, thisline); - (void) adm_print_attributes(my_data, entry->attributes); + if (retval = adm_print_attributes(my_data, entry->attributes)) { + free(my_data); + return retval; + } switch (entry->salt_type & 0xff) { case 0 : strcat(my_data, "Principal Salt Type (PST) = Version 5 Normal\n"); diff --git a/src/kadmin/server/adm_funcs.c b/src/kadmin/server/adm_funcs.c index 508c20953..1dfa0a419 100644 --- a/src/kadmin/server/adm_funcs.c +++ b/src/kadmin/server/adm_funcs.c @@ -169,7 +169,7 @@ OLDDECLARG(krb5_db_entry *, entry) extern krb5_flags NEW_ATTRIBUTES; if (!req_type) { /* New entry - initialize */ - memset((char *) entry, 0, sizeof(*entry)); + memset((char *) entry, 0, sizeof(krb5_db_entry)); entry->principal = (krb5_principal) principal; entry->kvno = KDB5_VERSION_NUM; entry->max_life = KDB5_MAX_TKT_LIFE; @@ -201,6 +201,11 @@ OLDDECLARG(krb5_db_entry *, entry) alt_key, &entry->alt_key); if (retval) { + if (entry->key.contents) { + memset((char *) entry->key.contents, 0, entry->key.length); + krb5_xfree(entry->key.contents); + entry->key.contents = 0; + } com_err("adm_modify_kdb", retval, "while encrypting alt_key for '%s'", newprinc); return(KADM_NO_ENCRYPT); @@ -209,12 +214,16 @@ OLDDECLARG(krb5_db_entry *, entry) if (retval = krb5_timeofday(&entry->mod_date)) { com_err("adm_modify_kdb", retval, "while fetching date"); - memset((char *) entry->key.contents, 0, entry->key.length); - memset((char *) entry->alt_key.contents, 0, entry->alt_key.length); - if (entry->key.contents) + if (entry->key.contents) { + memset((char *) entry->key.contents, 0, entry->key.length); krb5_xfree(entry->key.contents); - if (entry->alt_key.contents) + entry->key.contents = 0; + } + if (entry->alt_key.contents) { krb5_xfree(entry->alt_key.contents); + memset((char *) entry->alt_key.contents, 0, entry->alt_key.length); + entry->alt_key.contents = 0; + } return(KRB_ERR_GENERIC); } @@ -258,25 +267,34 @@ OLDDECLARG(krb5_db_entry *, entry) } else { if (retval = krb5_timeofday(&entry->last_pwd_change)) { com_err("adm_modify_kdb", retval, "while fetching date"); - memset((char *) entry->key.contents, 0, entry->key.length); - memset((char *) entry->alt_key.contents, 0, entry->alt_key.length); - if (entry->key.contents) + if (entry->key.contents) { + memset((char *) entry->key.contents, 0, entry->key.length); krb5_xfree(entry->key.contents); - if (entry->alt_key.contents) + entry->key.contents = 0; + } + if (entry->alt_key.contents) { + memset((char *) entry->alt_key.contents, 0, + entry->alt_key.length); krb5_xfree(entry->alt_key.contents); + entry->alt_key.contents = 0; + } return(5); } } retval = krb5_db_put_principal(entry, &one); - memset((char *) entry->key.contents, 0, entry->key.length); - if (entry->key.contents) + if (entry->key.contents) { + memset((char *) entry->key.contents, 0, entry->key.length); krb5_xfree(entry->key.contents); + entry->key.contents = 0; + } - memset((char *) entry->alt_key.contents, 0, entry->alt_key.length); - if (entry->alt_key.contents) + if (entry->alt_key.contents) { + memset((char *) entry->alt_key.contents, 0, entry->alt_key.length); krb5_xfree(entry->alt_key.contents); + entry->alt_key.contents = 0; + } if (retval) { com_err("adm_modify_kdb", retval, @@ -320,12 +338,15 @@ OLDDECLARG(krb5_db_entry *, entry) salt.salttype = salttype; + tempkey.contents = alttempkey.contents = 0; + retval = KRB_ERR_GENERIC; + switch (salttype) { case KRB5_KDB_SALTTYPE_NORMAL: if (retval = krb5_principal2salt(string_princ, &salt.saltdata)) { com_err("adm_enter_pwd_key", retval, "while converting principal to salt for '%s'", newprinc); - return(KRB_ERR_GENERIC); + goto cleanup; } altsalt.salttype = KRB5_KDB_SALTTYPE_V4; @@ -339,7 +360,7 @@ OLDDECLARG(krb5_db_entry *, entry) if (retval = krb5_principal2salt(string_princ, &altsalt.saltdata)) { com_err("adm_enter_pwd_key", retval, "while converting principal to altsalt for '%s'", newprinc); - return(KRB_ERR_GENERIC); + goto cleanup; } altsalt.salttype = KRB5_KDB_SALTTYPE_NORMAL; @@ -350,7 +371,7 @@ OLDDECLARG(krb5_db_entry *, entry) &salt.saltdata)) { com_err("adm_enter_pwd_key", retval, "while converting principal to salt for '%s'", newprinc); - return(KRB_ERR_GENERIC); + goto cleanup; } altsalt.salttype = KRB5_KDB_SALTTYPE_V4; @@ -365,7 +386,7 @@ OLDDECLARG(krb5_db_entry *, entry) &foo)) { com_err("adm_enter_pwd_key", retval, "while converting principal to salt for '%s'", newprinc); - return(KRB_ERR_GENERIC); + goto cleanup; } salt.saltdata = *foo; @@ -379,7 +400,7 @@ OLDDECLARG(krb5_db_entry *, entry) default: com_err("adm_enter_pwd_key", 0, "Don't know how to enter salt type %d", salttype); - return(KRB_ERR_GENERIC); + goto cleanup; } if (retval = krb5_string_to_key(&master_encblock, @@ -388,10 +409,8 @@ OLDDECLARG(krb5_db_entry *, entry) &pwd, &salt.saltdata)) { com_err("adm_enter_pwd_key", retval, - "while converting password to alt_key for '%s'", newprinc); - memset((char *) new_password, 0, sizeof(new_password)); /* erase it */ - krb5_xfree(salt.saltdata.data); - return(retval); + "while converting password to key for '%s'", newprinc); + goto cleanup; } if (retval = krb5_string_to_key(&master_encblock, @@ -401,10 +420,7 @@ OLDDECLARG(krb5_db_entry *, entry) &altsalt.saltdata)) { com_err("adm_enter_pwd_key", retval, "while converting password to alt_key for '%s'", newprinc); - krb5_xfree(salt.saltdata.data); - free(entry->alt_key.contents); - memset((char *) new_password, 0, sizeof(new_password)); /* erase it */ - return(retval); + goto cleanup; } memset((char *) new_password, 0, sizeof(new_password)); /* erase it */ @@ -419,10 +435,20 @@ OLDDECLARG(krb5_db_entry *, entry) &altsalt, entry); - memset((char *) tempkey.contents, 0, tempkey.length); - memset((char *) alttempkey.contents, 0, alttempkey.length); - if (entry->alt_key.contents) - free(entry->alt_key.contents); +cleanup: + if (salt.saltdata.data) + krb5_xfree(salt.saltdata.data); + if (altsalt.saltdata.data) + krb5_xfree(altsalt.saltdata.data); + if (tempkey.contents) { + memset((char *) tempkey.contents, 0, tempkey.length); + krb5_xfree(tempkey.contents); + } + if (alttempkey.contents) { + memset((char *) alttempkey.contents, 0, alttempkey.length); + krb5_xfree(alttempkey.contents); + } + memset((char *) new_password, 0, pwd.length); /* erase password */ return(retval); } @@ -456,7 +482,11 @@ krb5_ticket *client_creds; return(1); } - retval = krb5_unparse_name(newprinc, &composite_name); + if (retval = krb5_unparse_name(newprinc, &composite_name)) { + krb5_free_principal(newprinc); + krb5_db_free_principal(&entry, nprincs); + return retval; + } if (entry.salt_type == KRB5_KDB_SALTTYPE_V4) { entry.salt_type = KRB5_KDB_SALTTYPE_NORMAL; @@ -563,9 +593,8 @@ OLDDECLARG(krb5_db_entry *, entry) goto finish; } - retval = krb5_unparse_name(change_princ, &principal_name); - if (retval) - return retval; + if (retval = krb5_unparse_name(change_princ, &principal_name)) + goto finish; /* Modify Database */ retval = adm_modify_kdb("adm_enter_rnd_pwd_key", @@ -587,12 +616,10 @@ OLDDECLARG(krb5_db_entry *, entry) finish: - if(retval) { + if (tempkey->contents) { memset((char *) tempkey->contents, 0, tempkey->length); - return(retval); + krb5_free_keyblock(tempkey); } - memset((char *) tempkey->contents, 0, tempkey->length); - - return(0); + return(retval); } diff --git a/src/kadmin/server/adm_kadmin.c b/src/kadmin/server/adm_kadmin.c index abf38cad6..c553839b7 100644 --- a/src/kadmin/server/adm_kadmin.c +++ b/src/kadmin/server/adm_kadmin.c @@ -89,6 +89,7 @@ int *otype; /* Send Acknowledgement Reply to Client */ if (retval = krb5_write_message(&client_server_info.client_socket, &msg_data)){ + free(msg_data.data); syslog(LOG_ERR, "adm5_kadmin - Error Performing Acknowledgement Write: %s", error_message(retval)); @@ -283,7 +284,6 @@ int *otype; "Unknown or Non-Implemented Operation Type!", inet_ntoa(client_server_info.client_name.sin_addr)); syslog(LOG_AUTH | LOG_INFO, completion_msg); - free(completion_msg); retval = 255; goto send_last; } /* switch (request_type.oper_code) */ @@ -318,7 +318,6 @@ process_retval: inet_ntoa(client_server_info.client_name.sin_addr), kadmind_kadmin_response[retval]); syslog(LOG_AUTH | LOG_INFO, completion_msg); - free(completion_msg); goto send_last; default: @@ -327,7 +326,7 @@ process_retval: retbuf[2] = KUNKNOWNERR; retbuf[3] = '\0'; sprintf(completion_msg, "%s %s from %s FAILED", - "ksrvutil", + "kadmin", oper_type[1], inet_ntoa( client_server_info.client_name.sin_addr)); syslog(LOG_AUTH | LOG_INFO, completion_msg); @@ -359,6 +358,7 @@ send_last: /* Send Final Reply to Client */ if (retval = krb5_write_message(&client_server_info.client_socket, &msg_data)){ + free(msg_data.data); syslog(LOG_ERR, "adm5_kadmin - Error Performing Final Write: %s", error_message(retval)); return(1); diff --git a/src/kadmin/server/adm_nego.c b/src/kadmin/server/adm_nego.c index 43fa9962c..74a78cd39 100644 --- a/src/kadmin/server/adm_nego.c +++ b/src/kadmin/server/adm_nego.c @@ -208,6 +208,7 @@ OLDDECLARG(char *, new_passwd) clear_encodable_data(); com_err("adm_negotiate_key", ENOMEM, "for Additional Passwords"); + return(1); } strcpy((*next_passwd_phrase_element)->phrase->data, tmp_phrase); diff --git a/src/kadmin/server/adm_process.c b/src/kadmin/server/adm_process.c index 2eba71860..719fed671 100644 --- a/src/kadmin/server/adm_process.c +++ b/src/kadmin/server/adm_process.c @@ -81,7 +81,7 @@ OLDDECLARG(krb5_keyblock **, key) syslog(LOG_ERR, "cpw_keyproc %d while attempting to parse \"%s\"", client_server_info.name_of_service, retval); - return(0); + return(retval); } if (retval = krb5_db_get_principal(cpw_krb, &cpw_entry, @@ -89,7 +89,7 @@ OLDDECLARG(krb5_keyblock **, key) syslog(LOG_ERR, "cpw_keyproc %d while extracting %s entry", client_server_info.name_of_service, retval); - return(0); + return(retval); } if (!nprincs) return(0); @@ -99,7 +99,7 @@ OLDDECLARG(krb5_keyblock **, key) krb5_db_free_principal(&cpw_entry, nprincs); syslog(LOG_ERR, "cpw_keyproc: No Memory for server key"); close(client_server_info.client_socket); - return(0); + return(ENOMEM); } /* Extract the real kadmin/ keyblock */ @@ -112,7 +112,7 @@ OLDDECLARG(krb5_keyblock **, key) syslog(LOG_ERR, "cpw_keyproc: Cannot extract %s from master key", client_server_info.name_of_service); - exit(0); + exit(retval); } *key = realkey; -- 2.26.2