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

diff --git a/src/kadmin/client/ChangeLog b/src/kadmin/client/ChangeLog
new file mode 100644 (file)
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 fc67d2189f43321a38ed6152f18d6dd2e29290fc..f12480edc98b029371154e37436239b11b55e627 100644 (file)
--- 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 8a021d53ad345e5b6158dbc7f0fe3eb33cf5549b..a2d83673d6c25883d64dafe87f32f273ac3a6087 100644 (file)
--- 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 08ad01812617dd820ca60d6171c86dfdd384aca4..8655f5dce29e5746a1d3475fb1a2c18a9c46bde4 100644 (file)
--- 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 7e773d79338adf1ad5a2ff91666ebe5e3facbdbb..0d895b0bb974f47370cc840764581ae73a4dd19e 100644 (file)
--- 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 50600c6b3120f72e95d23da4d1fc634d1b06ea8f..e71cdd6baa071604acacdb9a189ef09cf1125a8c 100644 (file)
--- 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 a8c561e908d3fa6bcee6bde1d4bcc5550fb0b4be..6982037723c49f7752b89e0c0a14ea93879c2852 100644 (file)
--- 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 508c2095314ef6c5a74a7c745feb1e6a22f6adb1..1dfa0a419219ea05374584f04052ad093aac0116 100644 (file)
--- 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 abf38cad6793e0bf90973b0211476aed25a818e5..c553839b7de2e8ee865050dab8e14418cc6337e0 100644 (file)
--- 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 43fa9962c5ade51bd655b84b28e4af28733fae59..74a78cd395f833f3e21efbbc75bce97c13270373 100644 (file)
--- 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 2eba71860ae70defe511890cd6d98d47cf8b33c6..719fed6710643b9c821b109c8bf9920c2cb0f1b7 100644 (file)
--- 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;