From 7685f0567cf5c79d99737d34384a3122245dc533 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Mon, 4 May 2009 16:08:03 +0000 Subject: [PATCH] krb5_rd_rep could leak memory through its output parameter on error. Adjust the flow control so that *repl is NULL on error and the memory allocated by decode_krb5_ap_rep_enc_part is freed. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@22305 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/krb/rd_rep.c | 60 ++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/lib/krb5/krb/rd_rep.c b/src/lib/krb5/krb/rd_rep.c index 2d495ae8f..fe6c76375 100644 --- a/src/lib/krb5/krb/rd_rep.c +++ b/src/lib/krb5/krb/rd_rep.c @@ -73,49 +73,53 @@ krb5_rd_rep(krb5_context context, krb5_auth_context auth_context, const krb5_data *inbuf, krb5_ap_rep_enc_part **repl) { krb5_error_code retval; - krb5_ap_rep * reply; + krb5_ap_rep *reply = NULL; + krb5_ap_rep_enc_part *enc = NULL; krb5_data scratch; + *repl = NULL; + if (!krb5_is_ap_rep(inbuf)) return KRB5KRB_AP_ERR_MSG_TYPE; - /* decode it */ - - if ((retval = decode_krb5_ap_rep(inbuf, &reply))) + /* Decode inbuf. */ + retval = decode_krb5_ap_rep(inbuf, &reply); + if (retval) return retval; - /* put together an eblock for this encryption */ - + /* Put together an eblock for this encryption. */ scratch.length = reply->enc_part.ciphertext.length; - if (!(scratch.data = malloc(scratch.length))) { - krb5_free_ap_rep(context, reply); - return(ENOMEM); + scratch.data = malloc(scratch.length); + if (scratch.data == NULL) { + retval = ENOMEM; + goto clean_scratch; } - if ((retval = krb5_c_decrypt(context, auth_context->keyblock, - KRB5_KEYUSAGE_AP_REP_ENCPART, 0, - &reply->enc_part, &scratch))) + retval = krb5_c_decrypt(context, auth_context->keyblock, + KRB5_KEYUSAGE_AP_REP_ENCPART, 0, + &reply->enc_part, &scratch); + if (retval) goto clean_scratch; - /* now decode the decrypted stuff */ - retval = decode_krb5_ap_rep_enc_part(&scratch, repl); + /* Now decode the decrypted stuff. */ + retval = decode_krb5_ap_rep_enc_part(&scratch, &enc); if (retval) goto clean_scratch; - /* Check reply fields */ - if (((*repl)->ctime != auth_context->authentp->ctime) || - ((*repl)->cusec != auth_context->authentp->cusec)) { + /* Check reply fields. */ + if ((enc->ctime != auth_context->authentp->ctime) + || (enc->cusec != auth_context->authentp->cusec)) { retval = KRB5_MUTUAL_FAILED; goto clean_scratch; } - /* Set auth subkey */ - if ((*repl)->subkey) { + /* Set auth subkey. */ + if (enc->subkey) { if (auth_context->recv_subkey) { krb5_free_keyblock(context, auth_context->recv_subkey); auth_context->recv_subkey = NULL; } - retval = krb5_copy_keyblock(context, (*repl)->subkey, + retval = krb5_copy_keyblock(context, enc->subkey, &auth_context->recv_subkey); if (retval) goto clean_scratch; @@ -123,23 +127,27 @@ krb5_rd_rep(krb5_context context, krb5_auth_context auth_context, krb5_free_keyblock(context, auth_context->send_subkey); auth_context->send_subkey = NULL; } - retval = krb5_copy_keyblock(context, (*repl)->subkey, + retval = krb5_copy_keyblock(context, enc->subkey, &auth_context->send_subkey); if (retval) { krb5_free_keyblock(context, auth_context->send_subkey); auth_context->send_subkey = NULL; + goto clean_scratch; } - /* not used for anything yet */ - auth_context->negotiated_etype = (*repl)->subkey->enctype; + /* Not used for anything yet. */ + auth_context->negotiated_etype = enc->subkey->enctype; } - /* Get remote sequence number */ - auth_context->remote_seq_number = (*repl)->seq_number; + /* Get remote sequence number. */ + auth_context->remote_seq_number = enc->seq_number; + + *repl = enc; + enc = NULL; clean_scratch: memset(scratch.data, 0, scratch.length); - krb5_free_ap_rep(context, reply); + krb5_free_ap_rep_enc_part(context, enc); free(scratch.data); return retval; } -- 2.26.2