This patch addresses the issues raised in this ticket and ticket 5936

(a) In the case where 'cred_handle' != 'verifier_cred_handle'[1]
krb5_gss_accept_sec_context() leaks the 'cred_handle' in the success
case and the failure cases that result in returning from the function
prior to reaching the end of the function.

(b) The meaningful 'minor_status' return value is destroyed during the
cleanup operations.

The approach taken is to add a new 'exit:' label prior to the end of the
function through which all function returns after reaching the 'fail:'
label will goto.  After 'exit:', the 'cred_handle' will be released and
if there is a krb5_context 'context' to be freed, the error info will be
saved and krb5_free_context() will be called.

In the success case, the krb5_context is saved in the gss context and we
now set 'context' to NULL to prevent it from being freed.

In order to preserve the minor_status return code, a 'tmp_minor_status'
variable is added that is used after the 'fail:' label in calls to
krb5_gss_delete_sec_context() and krb5_gss_release_cred().


[1] If 'verifier_cred_handle' is non-NULL, then 'cred_handle' is set to
the value of 'verifier_cred_handle'.

ticket: 5442
tags: pullup

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

1 files changed, 19 insertions, 18 deletions

diff --git a/src/lib/gssapi/krb5/accept_sec_context.c b/src/lib/gssapi/krb5/accept_sec_context.c
index 9eccef243..bd53e29de 100644
--- a/src/lib/gssapi/krb5/accept_sec_context.c
+++ b/src/lib/gssapi/krb5/accept_sec_context.c
@@ -249,6 +249,7 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
    krb5_data option;
    const gss_OID_desc *mech_used = NULL;
    OM_uint32 major_status = GSS_S_FAILURE;
+   OM_uint32 tmp_minor_status;
    krb5_error krb_error_data;
    krb5_data scratch;
    gss_cred_id_t cred_handle = NULL;
@@ -905,13 +906,14 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
 
    if (!GSS_ERROR(major_status) && major_status != GSS_S_CONTINUE_NEEDED) {
        ctx->k5_context = context;
-       return(major_status);
+       context = NULL;
+       goto exit;
    }
 
    /* from here on is the real "fail" code */
 
    if (ctx)
-       (void) krb5_gss_delete_sec_context(minor_status, 
+       (void) krb5_gss_delete_sec_context(&tmp_minor_status, 
 					  (gss_ctx_id_t *) &ctx, NULL);
    if (deleg_cred) { /* free memory associated with the deleg credential */
        if (deleg_cred->ccache)
@@ -938,10 +940,9 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
    if (decode_req_message) {
        krb5_ap_req 	* request;
 	   
-       if (decode_krb5_ap_req(&ap_req, &request)) {
-	   krb5_free_context(context);
-	   return (major_status);
-       }
+       if (decode_krb5_ap_req(&ap_req, &request))
+	   goto exit;
+
        if (request->ap_options & AP_OPTS_MUTUAL_REQUIRED)
 	   gss_flags |= GSS_C_MUTUAL_FLAG;
        krb5_free_ap_req(context, request);
@@ -969,20 +970,16 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
        krb_error_data.server = cred->princ;
 
        code = krb5_mk_error(context, &krb_error_data, &scratch);
-       if (code) {
-	   krb5_free_context(context);
-	   return (major_status);
-       }
+       if (code)
+           goto exit;
 
        tmsglen = scratch.length;
        toktype = KG_TOK_CTX_ERROR;
 
        token.length = g_token_size(mech_used, tmsglen);
        token.value = (unsigned char *) xmalloc(token.length);
-       if (!token.value) {
-	   krb5_free_context(context);
-	   return (major_status);
-       }
+       if (!token.value) 
+           goto exit;
 
        ptr = token.value;
        g_make_token_header(mech_used, tmsglen, &ptr, toktype);
@@ -992,11 +989,15 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
 
        *output_token = token;
    }
+
+  exit:
    if (!verifier_cred_handle && cred_handle) {
-	   krb5_gss_release_cred(minor_status, &cred_handle);
+       krb5_gss_release_cred(&tmp_minor_status, &cred_handle);
+   }
+   if (context) {
+       if (major_status && *minor_status)
+           save_error_info(*minor_status, context);
+       krb5_free_context(context);
    }
-   if (major_status && *minor_status)
-       save_error_info(*minor_status, context);
-   krb5_free_context(context);
    return (major_status);
 }