From: Sam Hartman Date: Tue, 13 May 2003 17:05:27 +0000 (+0000) Subject: Fix memory leaks and double frees in preauth2.c X-Git-Tag: krb5-1.4-beta1~967 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=605623000c7c1bc01a82d42efbaab140d803bee2;p=krb5.git 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 58a85676d..709e2b390 100644 --- a/src/include/ChangeLog +++ b/src/include/ChangeLog @@ -1,3 +1,7 @@ +2003-05-13 Sam Hartman + + * k5-int.h: Add krb5int_copy_data_contents + 2003-05-08 Sam Hartman * 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 b9f8722c1..086b7df71 100644 --- 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 b6e2480e1..acce4eadb 100644 --- a/src/lib/krb5/krb/ChangeLog +++ b/src/lib/krb5/krb/ChangeLog @@ -1,3 +1,12 @@ +2003-05-13 Sam Hartman + + * 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 * 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 2899c5a88..1be2a2da5 100644 --- 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 2f426d721..3ccb6066f 100644 --- 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 5a9c1ba9e..3dadabc97 100644 --- 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); }