From b77645afb15ff28ca7468b53edaa61691f46def0 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Tue, 2 Feb 2010 00:37:33 +0000 Subject: [PATCH] Clean up error handling in krb5int_make_tgs_request_ext, closing some unlikely memory leaks. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@23682 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/krb/send_tgs.c | 101 +++++++++++++++--------------------- 1 file changed, 41 insertions(+), 60 deletions(-) diff --git a/src/lib/krb5/krb/send_tgs.c b/src/lib/krb5/krb/send_tgs.c index 2baba8a64..7f9b2dc5e 100644 --- a/src/lib/krb5/krb/send_tgs.c +++ b/src/lib/krb5/krb/send_tgs.c @@ -173,7 +173,7 @@ krb5int_make_tgs_request_ext(krb5_context context, krb5_error_code retval; krb5_kdc_req tgsreq; krb5_data *scratch, scratch2; - krb5_ticket *sec_ticket = 0; + krb5_ticket *sec_ticket = NULL; krb5_ticket *sec_ticket_arr[2]; krb5_timestamp time_now; krb5_pa_data **combined_padata = NULL; @@ -188,7 +188,7 @@ krb5int_make_tgs_request_ext(krb5_context context, * place holder for us to get credentials for the caller. */ if (!in_cred->ticket.length) - return(KRB5_NO_TKT_SUPPLIED); + return KRB5_NO_TKT_SUPPLIED; memset(&tgsreq, 0, sizeof(tgsreq)); @@ -200,7 +200,7 @@ krb5int_make_tgs_request_ext(krb5_context context, tgsreq.authorization_data.ciphertext.data = NULL; tgsreq.rtime = timestruct->renew_till; if ((retval = krb5_timeofday(context, &time_now))) - return(retval); + return retval; /* XXX we know they are the same size... */ *nonce = tgsreq.nonce = (krb5_int32)time_now; *timestamp = time_now; @@ -216,26 +216,24 @@ krb5int_make_tgs_request_ext(krb5_context context, /* need to encrypt it in the request */ if ((retval = encode_krb5_authdata(authorization_data, &scratch))) - goto send_tgs_error_1; - - if ((retval = krb5_encrypt_helper(context, local_subkey, - KRB5_KEYUSAGE_TGS_REQ_AD_SUBKEY, - scratch, - &tgsreq.authorization_data))) { - free(tgsreq.authorization_data.ciphertext.data); - krb5_free_data(context, scratch); - goto send_tgs_error_1; - } + goto cleanup; + retval = krb5_encrypt_helper(context, local_subkey, + KRB5_KEYUSAGE_TGS_REQ_AD_SUBKEY, + scratch, &tgsreq.authorization_data); krb5_free_data(context, scratch); + if (retval) + goto cleanup; } /* Get the encryption types list */ if (ktypes) { /* Check passed ktypes and make sure they're valid. */ for (tgsreq.nktypes = 0; ktypes[tgsreq.nktypes]; tgsreq.nktypes++) { - if (!krb5_c_valid_enctype(ktypes[tgsreq.nktypes])) - return KRB5_PROG_ETYPE_NOSUPP; + if (!krb5_c_valid_enctype(ktypes[tgsreq.nktypes])) { + retval = KRB5_PROG_ETYPE_NOSUPP; + goto cleanup; + } } tgsreq.ktype = (krb5_enctype *)ktypes; } else { @@ -246,7 +244,7 @@ krb5int_make_tgs_request_ext(krb5_context context, if (second_ticket) { if ((retval = decode_krb5_ticket(second_ticket, &sec_ticket))) - goto send_tgs_error_1; + goto cleanup; sec_ticket_arr[0] = sec_ticket; sec_ticket_arr[1] = 0; tgsreq.second_ticket = sec_ticket_arr; @@ -257,7 +255,7 @@ krb5int_make_tgs_request_ext(krb5_context context, /* encode the body; then checksum it */ if ((retval = encode_krb5_kdc_req_body(&tgsreq, &scratch))) - goto send_tgs_error_2; + goto cleanup; /* * Get an ap_req. @@ -265,19 +263,19 @@ krb5int_make_tgs_request_ext(krb5_context context, if ((retval = tgs_construct_tgsreq(context, scratch, in_cred, &scratch2, local_subkey))) { krb5_free_data(context, scratch); - goto send_tgs_error_2; + goto cleanup; } krb5_free_data(context, scratch); - tgsreq.padata = (krb5_pa_data **)calloc(2, sizeof(krb5_pa_data *)); + tgsreq.padata = k5alloc(2 * sizeof(krb5_pa_data *), &retval); if (tgsreq.padata == NULL) { free(scratch2.data); - goto send_tgs_error_2; + goto cleanup; } - tgsreq.padata[0] = (krb5_pa_data *)malloc(sizeof(krb5_pa_data)); + tgsreq.padata[0] = k5alloc(sizeof(krb5_pa_data), &retval); if (tgsreq.padata[0] == NULL) { free(scratch2.data); - goto send_tgs_error_2; + goto cleanup; } tgsreq.padata[0]->pa_type = KRB5_PADATA_AP_REQ; tgsreq.padata[0]->length = scratch2.length; @@ -297,29 +295,26 @@ krb5int_make_tgs_request_ext(krb5_context context, for (i = 0; padata[i]; i++) ; - tmp = (krb5_pa_data **)realloc(tgsreq.padata, - (i + 2) * sizeof(*combined_padata)); - if (tmp == NULL) - goto send_tgs_error_2; + tmp = realloc(tgsreq.padata, (i + 2) * sizeof(*combined_padata)); + if (tmp == NULL) { + retval = ENOMEM; + goto cleanup; + } tgsreq.padata = tmp; for (i = 0; padata[i]; i++) { krb5_pa_data *pa; - pa = tgsreq.padata[1 + i] = (krb5_pa_data *)malloc(sizeof(krb5_pa_data)); - if (tgsreq.padata == NULL) { - retval = ENOMEM; - goto send_tgs_error_2; - } + pa = tgsreq.padata[1 + i] = k5alloc(sizeof(krb5_pa_data), &retval); + if (tgsreq.padata == NULL) + goto cleanup; pa->pa_type = padata[i]->pa_type; pa->length = padata[i]->length; - pa->contents = (krb5_octet *)malloc(padata[i]->length); - if (pa->contents == NULL) { - retval = ENOMEM; - goto send_tgs_error_2; - } + pa->contents = k5alloc(padata[i]->length, &retval); + if (pa->contents == NULL) + goto cleanup; memcpy(pa->contents, padata[i]->contents, padata[i]->length); } tgsreq.padata[1 + i] = NULL; @@ -327,42 +322,28 @@ krb5int_make_tgs_request_ext(krb5_context context, if (pacb_fct != NULL) { if ((retval = (*pacb_fct)(context, local_subkey, &tgsreq, pacb_data))) - goto send_tgs_error_2; + goto cleanup; } /* the TGS_REQ is assembled in tgsreq, so encode it */ if ((retval = encode_krb5_tgs_req(&tgsreq, &scratch))) - goto send_tgs_error_2; - - /* now send request & get response from KDC */ - krb5_free_pa_data(context, tgsreq.padata); - tgsreq.padata = NULL; + goto cleanup; *request_data = *scratch; free(scratch); scratch = NULL; -send_tgs_error_2:; - if (tgsreq.padata) - krb5_free_pa_data(context, tgsreq.padata); - if (sec_ticket) - krb5_free_ticket(context, sec_ticket); + *subkey = local_subkey; + local_subkey = NULL; -send_tgs_error_1:; +cleanup: + krb5_free_pa_data(context, tgsreq.padata); + krb5_free_ticket(context, sec_ticket); if (ktypes == NULL) free(tgsreq.ktype); - if (tgsreq.authorization_data.ciphertext.data) { - memset(tgsreq.authorization_data.ciphertext.data, 0, - tgsreq.authorization_data.ciphertext.length); - free(tgsreq.authorization_data.ciphertext.data); - } - - if (retval) - krb5_free_keyblock(context, local_subkey); - else - *subkey = local_subkey; - + zapfree(tgsreq.authorization_data.ciphertext.data, + tgsreq.authorization_data.ciphertext.length); + krb5_free_keyblock(context, local_subkey); return retval; - } krb5_error_code -- 2.26.2