Fix memory leaks and double frees in preauth2.c

Ticket: 1470
Tags: pullup

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@15425 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/src/include/ChangeLog b/src/include/ChangeLog
index 58a85676d15b949404b9a7812bdb1d55ae3fbfac..709e2b3907d6ba7f02b646fa44cebdb64612beaf 100644 (file)
--- a/src/include/ChangeLog
+++ b/src/include/ChangeLog
@@ -1,3 +1,7 @@
+2003-05-13  Sam Hartman  <hartmans@mit.edu>
+
+       * k5-int.h: Add krb5int_copy_data_contents
+
 2003-05-08  Sam Hartman  <hartmans@mit.edu>
 
        * krb5.hin: Add prototype for krb5_c_string_to_key_with_params

diff --git a/src/include/k5-int.h b/src/include/k5-int.h
index b9f8722c1a7c4ba2d7ddbd8f925b3dd3cda4a12a..086b7df71423187379fac33cadbcaf6156e4e0e1 100644 (file)
--- a/src/include/k5-int.h
+++ b/src/include/k5-int.h
@@ -906,6 +906,8 @@ void krb5_free_etype_info
 /*
  * End "preauth.h"
  */
+krb5_error_code
+krb5int_copy_data_contents (krb5_context, const krb5_data *, krb5_data *);
 
 typedef krb5_error_code (*krb5_gic_get_as_key_fct)
     (krb5_context,

diff --git a/src/lib/krb5/krb/ChangeLog b/src/lib/krb5/krb/ChangeLog
index b6e2480e1e4a59dd28248f95ed25e4746c04ab7f..acce4eadb7b0f977e266bb22af2e60aae9a5ba92 100644 (file)
--- a/src/lib/krb5/krb/ChangeLog
+++ b/src/lib/krb5/krb/ChangeLog
@@ -1,3 +1,12 @@
+2003-05-13  Sam Hartman  <hartmans@mit.edu>
+
+       * get_in_tkt.c (krb5_get_init_creds): Free s2kparams
+
+       * preauth2.c (krb5_do_preauth): Fix memory management
+       (pa_salt): Use copy_data_contents
+
+       * copy_data.c (krb5int_copy_data_contents): New function
+
 2003-05-09  Sam Hartman  <hartmans@mit.edu>
 
        * preauth2.c: Patch from Sun to reorganize code   for handling

diff --git a/src/lib/krb5/krb/copy_data.c b/src/lib/krb5/krb/copy_data.c
index 2899c5a88ba2cc1e28aac95e60a4f3d2a200ed26..1be2a2da59384758a5d6ef32740d186f943f3f53 100644 (file)
--- a/src/lib/krb5/krb/copy_data.c
+++ b/src/lib/krb5/krb/copy_data.c
@@ -58,3 +58,25 @@ krb5_copy_data(krb5_context context, const krb5_data *indata, krb5_data **outdat
     *outdata = tempdata;
     return 0;
 }
+
+krb5_error_code 
+krb5int_copy_data_contents(krb5_context context, const krb5_data *indata, krb5_data *outdata)
+{
+    if (!indata) {
+       return EINVAL;
+    }
+    
+
+    outdata->length = indata->length;
+    if (outdata->length) {
+       if (!(outdata->data = malloc(outdata->length))) {
+           krb5_xfree(outdata);
+           return ENOMEM;
+       }
+       memcpy((char *)outdata->data, (char *)indata->data, outdata->length);
+    } else
+       outdata->data = 0;
+    outdata->magic = KV5M_DATA;
+
+    return 0;
+}

diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 2f426d721679c18bf24bfe98df3d029a411d90c6..3ccb6066f12bf0b7a5070567784bc5b181a41634 100644 (file)
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -1053,6 +1053,7 @@ cleanup:
     if (salt.data &&
        (!(options && (options->flags & KRB5_GET_INIT_CREDS_OPT_SALT))))
        krb5_xfree(salt.data);
+    krb5_free_data_contents(context, &s2kparams);
     if (as_reply)
        *as_reply = local_as_reply;
     else if (local_as_reply)

diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index 5a9c1ba9e0bca7d6b8d98a584980cdf920d3cf50..3dadabc9786a8350e5938d9bff4fd4389e1993db 100644 (file)
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -65,22 +65,11 @@ krb5_error_code pa_salt(krb5_context context,
 {
     krb5_data tmp;
 
-    /* screw the abstraction.  If there was a *reasonable* copy_data,
-       I'd use it.  But I'm inside the library, which is the twilight
-       zone of source code, so I can do anything. */
-
+    tmp.data = in_padata->contents;
     tmp.length = in_padata->length;
-    if (tmp.length) {
-       if ((tmp.data = malloc(tmp.length)) == NULL)
-           return ENOMEM;
-       memcpy(tmp.data, in_padata->contents, tmp.length);
-    } else {
-       tmp.data = NULL;
-    }
-
-    *salt = tmp;
-
-    /* assume that no other salt was allocated */
+    krb5_free_data_contents(context, salt);
+    krb5int_copy_data_contents(context, &tmp, salt);
+    
 
     if (in_padata->pa_type == KRB5_PADATA_AFS3_SALT)
        salt->length = SALT_TYPE_AFS_LENGTH;
@@ -842,7 +831,7 @@ krb5_do_preauth(krb5_context context,
 {
     int h, i, j, out_pa_list_size;
     int seen_etype_info2 = 0;
-    krb5_pa_data *out_pa, **out_pa_list;
+    krb5_pa_data *out_pa = NULL, **out_pa_list = NULL;
     krb5_data scratch;
     krb5_etype_info etype_info = NULL;
     krb5_error_code ret;
@@ -900,11 +889,7 @@ krb5_do_preauth(krb5_context context,
                scratch.data = (char *) in_padata[i]->contents;
                ret = decode_krb5_etype_info(&scratch, &etype_info);
                if (ret) {
-                   if (out_pa_list) {
-                       out_pa_list[out_pa_list_size++] = NULL;
-                       krb5_free_pa_data(context, out_pa_list);
-                   }
-                   return ret;
+                 goto cleanup;
                }
                if (etype_info[0] == NULL) {
                    krb5_free_etype_info(context, etype_info);
@@ -930,18 +915,32 @@ krb5_do_preauth(krb5_context context,
                    }
                }
                if (!etype_found) {
-                   if (valid_etype_found)
+                 if (valid_etype_found) {
                        /* supported enctype but not requested */
-                       return KRB5_CONFIG_ETYPE_NOSUPP;
-                   else
-                       /* unsupported enctype */
-                       return KRB5_PROG_ETYPE_NOSUPP;
+                   ret =  KRB5_CONFIG_ETYPE_NOSUPP;
+                   goto cleanup;
+                 }
+                 else {
+                   /* unsupported enctype */
+                   ret =  KRB5_PROG_ETYPE_NOSUPP;
+                   goto cleanup;
+                 }
 
                }
-               salt->data = (char *) etype_info[l]->salt;
-               salt->length = etype_info[l]->length;
+               scratch.data = (char *) etype_info[l]->salt;
+               scratch.length = etype_info[l]->length;
+               krb5_free_data_contents(context, salt);
+               if (scratch.length = KRB5_ETYPE_NO_SALT) 
+                 salt->data = NULL;
+               else
+                   if ((ret = krb5int_copy_data_contents( context, &scratch, salt)) != 0)
+                 goto cleanup;
                *etype = etype_info[l]->etype;
-               *s2kparams = etype_info[l]->s2kparams;
+               krb5_free_data_contents(context, s2kparams);
+               if ((ret = krb5int_copy_data_contents(context,
+                                                     &etype_info[l]->s2kparams,
+                                                     s2kparams)) != 0)
+                 goto cleanup;
 #ifdef DEBUG
                for (j = 0; etype_info[j]; j++) {
                    krb5_etype_info_entry *e = etype_info[j];
@@ -972,13 +971,7 @@ krb5_do_preauth(krb5_context context,
                                                   salt, s2kparams, etype, as_key,
                                                   prompter, prompter_data,
                                                   gak_fct, gak_data)))) {
-                       if (out_pa_list) {
-                           out_pa_list[out_pa_list_size++] = NULL;
-                           krb5_free_pa_data(context, out_pa_list);
-                       }
-                       if (etype_info)
-                           krb5_free_etype_info(context, etype_info);
-                       return(ret);
+                     goto cleanup;
                    }
 
                    if (out_pa) {
@@ -986,18 +979,22 @@ krb5_do_preauth(krb5_context context,
                            if ((out_pa_list =
                                 (krb5_pa_data **)
                                 malloc(2*sizeof(krb5_pa_data *)))
-                               == NULL)
-                               return(ENOMEM);
+                               == NULL) {
+                             ret = ENOMEM;
+                             goto cleanup;
+                           }
                        } else {
                            if ((out_pa_list =
                                 (krb5_pa_data **)
                                 realloc(out_pa_list,
                                         (out_pa_list_size+2)*
                                         sizeof(krb5_pa_data *)))
-                               == NULL)
-                               /* XXX this will leak the pointers which
+                               == NULL) {
+                             /* XXX this will leak the pointers which
                                   have already been allocated.  oh well. */
-                               return(ENOMEM);
+                             ret = ENOMEM;
+                             goto cleanup;
+                           }
                        }
                        
                        out_pa_list[out_pa_list_size++] = out_pa;
@@ -1013,6 +1010,16 @@ krb5_do_preauth(krb5_context context,
        out_pa_list[out_pa_list_size++] = NULL;
 
     *out_padata = out_pa_list;
-
+    if (etype_info)
+      krb5_free_etype_info(context, etype_info);
+    
     return(0);
+ cleanup:
+    if (out_pa_list) {
+      out_pa_list[out_pa_list_size++] = NULL;
+      krb5_free_pa_data(context, out_pa_list);
+    }
+    if (etype_info)
+      krb5_free_etype_info(context, etype_info);
+    return (ret);
 }