Received: from biscayne-one-station.mit.edu (BISCAYNE-ONE-STATION.MIT.EDU [18.7.7.80]) by krbdev.mit.edu (8.9.3p2) with ESMTP id RAA03805; Mon, 26 Dec 2005 17:04:45 -0500 (EST)
Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by biscayne-one-station.mit.edu (8.12.4/8.9.2) with ESMTP id jBQM4isT024624 for <rt@krbdev.MIT.EDU>; Mon, 26 Dec 2005 17:04:44 -0500 (EST)
Received: from cathode-dark-space.mit.edu (CATHODE-DARK-SPACE.MIT.EDU [18.18.1.96]) (authenticated bits=56) (User authenticated as tlyu@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.1/8.12.4) with ESMTP id jBQM4gZl021184 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT) for <rt@krbdev.MIT.EDU>; Mon, 26 Dec 2005 17:04:43 -0500 (EST)
Received: (from tlyu@localhost) by cathode-dark-space.mit.edu (8.12.9) id jBQM4glm001631; Mon, 26 Dec 2005 17:04:42 -0500 (EST)
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #3313] doublefree in gc_frm_kdc.c
References: <rt-3313-15330.7.51503416397078@krbdev.mit.edu>
From: Tom Yu <tlyu@MIT.EDU>
Date: Mon, 26 Dec 2005 17:04:42 -0500
In-Reply-To: <rt-3313-15330.7.51503416397078@krbdev.mit.edu> (Sam Hartman via's message of "Sun, 25 Dec 2005 22:00:25 -0500 (EST)")
Message-Id: <ldvvexb7bwl.fsf@cathode-dark-space.mit.edu>
Lines: 84
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Spam-Score: 1.217
X-Spam-Level: * (1.217)
X-Spam-Flag: NO
X-Scanned-BY: MIMEDefang 2.42
RT-Send-Cc: 
X-RT-Original-Encoding: us-ascii
Content-Length: 2396

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 */