MITKRB5-SA-2007-005: kadmind vulnerable to buffer overflow
lhinman at wareonearth.com
Wed Jun 27 21:03:12 EDT 2007
On Jun 27, 2007, at 3:27 PM, Mike Friedman wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> On Wed, 27 Jun 2007 at 13:28 (-0500), Lee Hinman wrote:
>> Just a little suggestion on your patch. Calling error_message
>> (ret.code) when ret.code == 0 may cause your output to be
>> something like "Unknown error: 0". It will depend on what your
>> libc does when you call sterror(0). Previously it would print out
>> "success". The change below restores that behavior.
> I guess you're referring to Russ Allbery's patch.
> Maybe I'm missing something, but I don't see your proposed change;
> what you included in your email seems to be just Russ's patch as-is.
Sorry, not sure what I did. Hopefully this will come through OK.
> Are you saying that 'error_message(ret.code)' should be replaced
> with something else, because the test for (ret.code == 0) is not
> always reliable as an indicator of success? If so, what should be
> used instead?
ret.code==0 is a reliable indicator for success but in one place
ret.code is being passed to error_message without first checking to
see if ret.code==0 (so a success). And error_message doesn't know
how to handle an error code of 0, so it passes it on to libc, and
sterror may or may not know what to do with an error code of 0.
So after applying Russ Allbery's patch, my suggestion is:
diff -Naur src.orig/kadmin/server/server_stubs.c src.patched/kadmin/
--- src.orig/kadmin/server/server_stubs.c 2007-06-27
+++ src.patched/kadmin/server/server_stubs.c 2007-06-27
@@ -583,7 +583,8 @@
"%.*s%s to %.*s%s, %s, "
"client=%.*s%s, service=%.*s%s, addr=%s",
tlen1, prime_arg1, tdots1,
- tlen2, prime_arg2, tdots2, error_message
+ tlen2, prime_arg2, tdots2,
+ ((ret.code==0) ? "success" : error_message(ret.code)),
clen, client_name.value, cdots,
slen, service_name.value, sdots,
So if ret.code==0 use "success" for the sting otherwise call
error_message with ret.code as the argument and use the return value
for the string.
More information about the krbdev