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

11 files changed, 118 insertions, 67 deletions

diff --git a/src/kadmin/client/ChangeLog b/src/kadmin/client/ChangeLog
new file mode 100644
index 0000000..56454ba
--- /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 fc67d21..f12480e 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 8a021d5..a2d8367 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 08ad018..8655f5d 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 7e773d7..0d895b0 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 50600c6..e71cdd6 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 a8c561e..6982037 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 <krb5/kdb.h>
 #include <krb5/kdb_dbm.h>
 
-#include <com_err.h>
 #include <stdio.h>
 
 #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 508c209..1dfa0a4 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 abf38ca..c553839 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 43fa996..74a78cd 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 2eba718..719fed6 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/<realm> 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;