[krbdev.mit.edu #3313] doublefree in gc_frm_kdc.c

[Tom Yu via RT]

I have come up with the following improved patch.  It should take care
of a few double-free situations not caught by Jeff's patch, as well as
some memory leaks introduced by Jeff's patch.  I'm still not convinced
that certain improbable error conditions won't result in additional
problems, but I think they're limited to null pointer derefs caused by
pre-existing code in the function.

Index: gc_frm_kdc.c
===================================================================
--- gc_frm_kdc.c	(revision 17577)
+++ gc_frm_kdc.c	(working copy)
@@ -231,13 +231,14 @@
 		goto cleanup;
 
 	    otgt = tgt;
-	    free_otgt = 1;
+	    free_otgt = free_tgt;
 	    free_tgt = 0;
 
 	    retval = krb5_cc_retrieve_cred(context, ccache, retr_flags,
 					   &tgtq, &tgt);
 	    if (retval == 0) {
-	        krb5_free_cred_contents(context, &otgt);
+		if (free_otgt)
+		    krb5_free_cred_contents(context, &otgt);
 		free_otgt = 0;
 	        free_tgt = 1;
 		/* We are now done - proceed to got/finally have tgt */
@@ -250,8 +251,8 @@
 		/* with current tgt.                              */
 		/* Copy back in case invalided */
 		tgt = otgt;
+		free_tgt = free_otgt;
 		free_otgt = 0;
-		free_tgt = 1;
 		if (!krb5_c_valid_enctype(tgt.keyblock.enctype)) {
 		    retval = KRB5_PROG_ETYPE_NOSUPP;
 		    goto cleanup;
@@ -305,7 +306,7 @@
 			    goto cleanup;
 
 			otgt = tgt;
-			free_otgt = 1;
+			free_otgt = free_tgt;
 			free_tgt = 0;
 			retval = krb5_cc_retrieve_cred(context, ccache,
 							    retr_flags,
@@ -324,8 +325,8 @@
 			    /* not in the cache so try and get one with our current tgt. */
   
 			    tgt = otgt;
+			    free_tgt = free_otgt;
 			    free_otgt = 0;
-			    free_tgt = 1;
 			    if (!krb5_c_valid_enctype(tgt.keyblock.enctype)) {
 				retval = KRB5_PROG_ETYPE_NOSUPP;
 				goto cleanup;
@@ -363,8 +364,11 @@
 				krb5_free_cred_contents(context, &otgt);
 				free_otgt = 0;
 			    }
+			    if (free_tgt)
+				krb5_free_cred_contents(context, &tgt);
 	      
 			    tgt = *ret_tgts[ntgts++];
+			    free_tgt = 0;
 			} 
           
 			/* got one as close as possible, now start all over */
@@ -413,12 +417,11 @@
 		krb5_free_creds(context, tgtr);
 		tgtr = NULL;
 
-		if (free_tgt) {
+		if (free_tgt)
 		    krb5_free_cred_contents(context, &tgt);
-		    free_tgt = 0;
-		}
 
 		tgt = *ret_tgts[ntgts++];
+		free_tgt = 0;
 
 		/* we're done if it is the target */