[3049] in Kerberos-V5-bugs
pending/699: [papowell@astart.com] Kerberos 5, please forward
daemon@ATHENA.MIT.EDU (Mike Whitson)
Mon Mar 8 21:20:07 1999
Resent-From: gnats@rt-11.MIT.EDU (GNATS Management)
Resent-To: gnats-admin@rt-11.MIT.EDU
Resent-Reply-To: krb5-bugs@MIT.EDU, Mike Whitson <mwhitson@MIT.EDU>
Date: 08 Mar 1999 21:19:31 -0500
From: Mike Whitson <mwhitson@MIT.EDU>
To: krb5-bugs@MIT.EDU
>Number: 699
>Category: pending
>Synopsis: [papowell@astart.com] Kerberos 5, please forward
>Confidential: yes
>Severity: serious
>Priority: medium
>Responsible: gnats-admin
>State: open
>Class: sw-bug
>Submitter-Id: unknown
>Arrival-Date: Mon Mar 08 21:20:00 EST 1999
>Last-Modified:
>Originator:
>Organization:
>Release:
>Environment:
>Description:
>How-To-Repeat:
>Fix:
>Audit-Trail:
>Unformatted:
------- Start of forwarded message -------
Date: Mon, 8 Mar 1999 17:52:19 -0800 (PST)
From: papowell@astart.com
Message-Id: <199903090152.RAA15439@astart4.astart.com>
To: mwhitson@MIT.EDU
Subject: Kerberos 5, please forward
I seem to have misfiled/misplaced the email message you
sent to me about the folks working on Kerberos 5. Apparently some bugs
are still in the Kerberos 5 library, especially in the routines
that I use.
File: ./lib/krb5/krb/sendauth.c
Problem: wrong value being returned on function exit.
Problem: dead (unreferenced) malloced() values are not freed on exit.
Original code, approximately line 213:
retval = 0; /* Normal return */
if (out_creds) {
*out_creds = credsp;
}
out_creds is a pointer to where you want the credentials
returned. At line 147 we have the code:
if (!in_creds->ticket.length) {
if ((retval = krb5_get_credentials(context, 0,
use_ccache, in_creds, &credsp)))
goto error_return;
credspout = credsp; <<<<--- value to be returned
} else {
credsp = in_creds;
}
So credspout is set to credsp, saving credsp's value
so that later assignments to credsp does not change it and lose the
returned credentials in the process.
I think what you really want is:
retval = 0; /* Normal return */
if (out_creds) {
*out_creds = credspout;
}
Now, if you cast your eye down a little further, you will discover
that you have:
error_return:
krb5_free_cred_contents(context, &creds);
if (!out_creds && credspout)
* krb5_free_creds(context, credspout);
* if (!ccache && use_ccache)
* krb5_cc_close(context, use_ccache);
return(retval);
}
The lines marked with * have the interesting side effect of
freeing the credspout value, which in the original code has
the sometimes disastrous effects of freeing the return value as well.
I believe that the correct code should be:
error_return:
krb5_free_cred_contents(context, &creds);
! if(out_creds == 0) krb5_free_creds(context, credspout);
! if (!ccache && use_ccache) krb5_cc_close(context, use_ccache);
return(retval);
}
Note that we now free creds - which is ok as it is not the credspout
value at this point.
If there is no out_creds value, we free credspout, eliminating a memory
leak.
Finally, we free close and free up stuff if we are not caching it.
Here is a diff -c of the code.
Enjoy.
Patrick Powell Astart Technologies,
papowell@astart.com 9475 Chesapeake Drive, Suite D,
Network and System San Diego, CA 92123
Consulting 619-874-6543 FAX 619-279-8424
LPRng - Print Spooler (http://www.astart.com)
***************
*** 229,242 ****
}
retval = 0; /* Normal return */
if (out_creds) {
! *out_creds = credspout;
}
error_return:
krb5_free_cred_contents(context, &creds);
! if(out_creds == 0) krb5_free_creds(context, credspout);
! if (!ccache && use_ccache) krb5_cc_close(context, use_ccache);
return(retval);
}
! #endif
--- 213,228 ----
}
retval = 0; /* Normal return */
if (out_creds) {
! *out_creds = credsp;
}
error_return:
krb5_free_cred_contents(context, &creds);
! if (!out_creds && credspout)
! krb5_free_creds(context, credspout);
! if (!ccache && use_ccache)
! krb5_cc_close(context, use_ccache);
return(retval);
}
------- End of forwarded message -------
