| Date: | Sat, 24 May 2003 08:32:24 -0400 (EDT) |
| From: | Ezra Peisach <epeisach@MIT.EDU> |
| To: | krb5-bugs@MIT.EDU |
| Subject: | API (inadvertant?) change in krb5_get_in_tkt_with_password leads to memory leaks |
There has been an undocumented API change in 1.3 with
krb5_get_in_tkt_with_password.
This function has the unfortunate API of using a krb5_creds * for both
input and output. In particular the creds->server and client are used on
input.
This function was recently changed to invoke krb5_get_init_creds.
In get_in_tkt.c about line 1040 there is the following comment and code:
/* XXX this should be inside stash_as_reply, but as long as
get_in_tkt is still around using that arg as an in/out, I can't
do that */
memset(creds, 0, sizeof(*creds));
if ((ret = stash_as_reply(context, time_now, &request, local_as_reply,
creds, NULL)))
stash_as_reply will copy over the client and server creds - if they are
not set (which is okay).
It is the memset that can lead to memory leaks - as it clears the
pointers to the client and server.
What to do to fix this?
a) Don't do the memset....
b) memset all but the client and server
c) Modify krb5_get_in_tkt_with_password and krb5_get_in_tkt_with_keytab
(and possibly others) to save a pointer to the client and server
portions so they can krb5_free_principal on return from init_creds and
then restore the pointers.
d) Change krb5_get_in_tkt_with_password and krb5_get_in_tkt_with_keytab
back to not use krb5_get_init_creds.
In terms of code reuse - I think (c) might be the best option as
changes (a) and (b) would make a change in krb5_get_init_creds from the
1.2 series....
(for those wanting to test the code bug - tests/hammer/kdc5_hammer
currently leaks memory in this fashion - when it did not in the past)
I believe this needs to be fixed before 1.3 goes out.
Ezra