Ticket History #1009: kadmin_parse_name() loops forever with, e.g., '@SOME.REALM'

From Nicolas.Williams@ubsw.com Mon Nov 5 13:55:35 2001
Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU [18.72.0.53])
by rt-11.mit.edu (8.9.3/8.9.3) with ESMTP id NAA11490
for <bugs@RT-11.mit.edu>; Mon, 5 Nov 2001 13:55:34 -0500 (EST)
Received: from gate2.stm.ubswarburg.com (gate2.stm.ubswarburg.com [151.191.1.12])
by fort-point-station.mit.edu (8.9.2/8.9.2) with ESMTP id NAA20532
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 13:55:34 -0500 (EST)
Received: (from smap@localhost)
by gate2.stm.ubswarburg.com (8.8.8/8.8.8) id NAA15022
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 13:55:32 -0500 (EST)
Received: from <Nicolas.Williams@ubsw.com> (thirteen.ubswarburg.com [192.168.0.7]) by gate2 via smap (V2.0/ubsw)
id xma014938; Mon, 5 Nov 2001 13:55:07 -0500
Received: from sm0p9035pos.stm.swissbank.com (virscan4 [192.168.0.7])
by virscan4.swissbank.com (8.8.8/8.8.8) with ESMTP id NAA22655
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 13:56:39 -0500 (EST)
Received: from sm2p1386swk.stm.swissbank.com (sm2p1386swk.stm.swissbank.com [151.191.93.86])
by sm0p9035pos.stm.swissbank.com (8.8.8/8.8.8) with ESMTP id NAA00172
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 13:55:06 -0500 (EST)
Received: (willian@localhost) by sm2p1386swk.stm.swissbank.com (8.9.3+Sun/8.6.12) id NAA25852 for krb5-bugs@mit.edu; Mon, 5 Nov 2001 13:55:06 -0500 (EST)
Message-Id: <20011105135506.L26615@wdr.com>
Date: Mon, 5 Nov 2001 13:55:06 -0500
From: Nicolas Williams <Nicolas.Williams@ubsw.com>
To: krb5-bugs@mit.edu
In-Reply-To: <200111051542.KAA22169@sm0d1989cmp.stm.swissbank.com>; from Nicolas Williams on Mon, Nov 05, 2001 at 10:42:32AM -0500
Subject: Re: Infinite loop in src/kadmin/cli/kadmin.c:kadmin_parse_name()
References: <200111051542.KAA22169@sm0d1989cmp.stm.swissbank.com>

Ooops. The patch is a bit more involved, actually... The else clause of
that if statement does "cp = strchr(cp, '@');" and it really should read
"cp = strchr(cp + 1, '@');".

Of course, why kadmin.local would use a different function that the
rest of the krb5 stuff, including the kadmin client cli, baffles me.
Particularly when krb5_parse_name() does everything that
kadmin_parse_name() would...

So perhaps the better patch would be to remove kadmin_parse_name()
altogether.

Cheers,

Nico

On Mon, Nov 05, 2001 at 10:42:32AM -0500, Nicolas Williams wrote:
--

Visit our website at http://www.ubswarburg.com

This message contains confidential information and is intended only
for the individual named. If you are not the named addressee you
should not disseminate, distribute or copy this e-mail. Please
notify the sender immediately by e-mail if you have received this
e-mail by mistake and delete this e-mail from your system.

E-mail transmission cannot be guaranteed to be secure or error-free
as information could be intercepted, corrupted, lost, destroyed,
arrive late or incomplete, or contain viruses. The sender therefore
does not accept liability for any errors or omissions in the contents
of this message which arise as a result of e-mail transmission. If
verification is required please request a hard-copy version. This
message is provided for informational purposes and should not be
construed as a solicitation or offer to buy or sell any securities or
related financial instruments.

From Nicolas.Williams@ubsw.com Mon Nov 5 10:44:14 2001
Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU [18.72.0.53])
by rt-11.mit.edu (8.9.3/8.9.3) with ESMTP id KAA11079
for <bugs@RT-11.mit.edu>; Mon, 5 Nov 2001 10:44:14 -0500 (EST)
Received: from gate.stm.swissbank.com (gate.stm.ubswarburg.com [151.191.1.10])
by fort-point-station.mit.edu (8.9.2/8.9.2) with ESMTP id KAA04146
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 10:44:13 -0500 (EST)
Received: (from smap@localhost)
by gate.stm.swissbank.com (8.8.8/8.8.8) id KAA18503
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 10:46:08 -0500 (EST)
Received: from <Nicolas.Williams@ubsw.com> (twelve.ubswarburg.com [192.168.0.6]) by gate via smap (V2.0)
id xma017897; Mon, 5 Nov 2001 10:44:51 -0500
Received: from sm0p9035pos.stm.swissbank.com (virscan3 [192.168.0.6])
by virscan3.swissbank.com (8.8.8/8.8.8) with ESMTP id KAA11168
for <krb5-bugs@mit.edu>; Mon, 5 Nov 2001 10:42:53 -0500 (EST)
Received: from sm0d1989cmp.stm.swissbank.com (sm0d1989cmp.stm.swissbank.com [151.191.119.89])
by sm0p9035pos.stm.swissbank.com (8.8.8/8.8.8) with ESMTP id KAA19599;
Mon, 5 Nov 2001 10:42:32 -0500 (EST)
Received: (willian@localhost) by sm0d1989cmp.stm.swissbank.com (8.8.8+Sun/8.6.12) id KAA22169; Mon, 5 Nov 2001 10:42:32 -0500 (EST)
Message-Id: <200111051542.KAA22169@sm0d1989cmp.stm.swissbank.com>
Date: Mon, 5 Nov 2001 10:42:32 -0500 (EST)
From: Nicolas Williams <Nicolas.Williams@ubsw.com>
Reply-To: Nicolas.Williams@ubsw.com
To: krb5-bugs@mit.edu
Subject: Infinite loop in src/kadmin/cli/kadmin.c:kadmin_parse_name()
X-Send-Pr-Version: 3.99

UBS Warburg / Perot Systems Corp.
--
Ultra-SPARC, Solaris 2.6
System: SunOS sm0d1989cmp 5.6 Generic_105181-12 sun4u sparc SUNW,Ultra-2
Architecture: sun4

Line 149 of src/kadmin/cli/kadmin.c should read:

if (cp == fullname || *(cp - 1) != '\\')

instead of:

if (cp - fullname && *(cp - 1) != '\\')

The expression (cp - fullname) is always true except when the
name being parsed starts with '@' and therefore is useless.

I believe the break condition was meant to be (cp == fullname || ...).

kadmin.local -q "ank -randkey @SWISSBANK.COM"
Here's the unified patch:

Index: PER-122.19/kadmin/cli/kadmin.c
--- PER-122.19/kadmin/cli/kadmin.c Wed, 14 Mar 2001 14:20:32 -0500
+++ PER-122.19(w)/kadmin/cli/kadmin.c Mon, 05 Nov 2001 10:39:06 -0500
@@ -146,7 +146,7 @@
strcpy(fullname, name);
cp = strchr(fullname, '@');
while (cp) {
- if (cp - fullname && *(cp - 1) != '\\')
+ if (cp == fullname || *(cp - 1) != '\\')
break;
else
cp = strchr(cp, '@');

Visit our website at http://www.ubswarburg.com

This message contains confidential information and is intended only
for the individual named. If you are not the named addressee you
should not disseminate, distribute or copy this e-mail. Please
notify the sender immediately by e-mail if you have received this
e-mail by mistake and delete this e-mail from your system.

E-mail transmission cannot be guaranteed to be secure or error-free
as information could be intercepted, corrupted, lost, destroyed,
arrive late or incomplete, or contain viruses. The sender therefore
does not accept liability for any errors or omissions in the contents
of this message which arise as a result of e-mail transmission. If
verification is required please request a hard-copy version. This
message is provided for informational purposes and should not be
construed as a solicitation or offer to buy or sell any securities or
related financial instruments.


From: Nicolas Williams <Nicolas.Williams@ubsw.com>
To: krb5-bugs@mit.edu, krb5-unassigned@rt-11.mit.edu
Cc: Subject: Re: krb5-admin/1009: Infinite loop in src/kadmin/cli/kadmin.c:kadmin_parse_name()
Date: Mon, 5 Nov 2001 13:55:29 -0500

Ooops. The patch is a bit more involved, actually... The else clause of
that if statement does "cp = strchr(cp, '@');" and it really should read
"cp = strchr(cp + 1, '@');".

Of course, why kadmin.local would use a different function that the
rest of the krb5 stuff, including the kadmin client cli, baffles me.
Particularly when krb5_parse_name() does everything that
kadmin_parse_name() would...

So perhaps the better patch would be to remove kadmin_parse_name()
altogether.

Cheers,

Nico


On Mon, Nov 05, 2001 at 10:45:00AM -0500, krb5-bugs@mit.edu wrote:
--

Visit our website at http://www.ubswarburg.com

This message contains confidential information and is intended only
for the individual named. If you are not the named addressee you
should not disseminate, distribute or copy this e-mail. Please
notify the sender immediately by e-mail if you have received this
e-mail by mistake and delete this e-mail from your system.

E-mail transmission cannot be guaranteed to be secure or error-free
as information could be intercepted, corrupted, lost, destroyed,
arrive late or incomplete, or contain viruses. The sender therefore
does not accept liability for any errors or omissions in the contents
of this message which arise as a result of e-mail transmission. If
verification is required please request a hard-copy version. This
message is provided for informational purposes and should not be
construed as a solicitation or offer to buy or sell any securities or
related financial instruments.


From: Nicolas Williams <Nicolas.Williams@ubsw.com>
To: krb5-bugs@mit.edu, krb5-unassigned@rt-11.mit.edu
Cc: Subject: Re: krb5-admin/1009: Infinite loop in src/kadmin/cli/kadmin.c:kadmin_parse_name()
Date: Mon, 5 Nov 2001 17:01:09 -0500

Oh hell, that code was so full of bugs... I understand why there's a
kadmin_parse_name() now -- so you can use a different default realm than
the one specified in /etc/krb5.conf, i.e., kadmin -r FOO.ORG...

The bugs in the old code:

1) "if (cp - fullname && *(cp - 1) != '\\')"
2) "cp = strchr(cp, '@');"
3) "if (cp == NULL)"

Here's a better, tested patch, with a much simpler loop, and no strchr
evil. Granted, this will require revisiting when the
internationalization stuff for Kerberos is decided... But that's
life...

Index: PER-122.19/kadmin/cli/kadmin.c
--- PER-122.19/kadmin/cli/kadmin.c Wed, 14 Mar 2001 14:20:32 -0500
+++ PER-122.19(w)/kadmin/cli/kadmin.c Mon, 05 Nov 2001 16:53:45 -0500
@@ -140,18 +140,18 @@
krb5_error_code retval;

/* assumes def_realm is initialized! */
- fullname = (char *)malloc(strlen(name) + 1 + strlen(def_realm) + 1);
+ cp = fullname = (char *)malloc(strlen(name) + 1 + strlen(def_realm) + 1);
if (fullname == NULL)
return ENOMEM;
strcpy(fullname, name);
- cp = strchr(fullname, '@');
- while (cp) {
- if (cp - fullname && *(cp - 1) != '\\')
- break;
- else
- cp = strchr(cp, '@');
+ while (*cp) {
+ if (*cp == '@')
+ break;
+ if (*cp == '\\' && *(cp+1))
+ cp++;
+ cp++;
}
- if (cp == NULL) {
+ if (*cp == '\0') {
strcat(fullname, "@");
strcat(fullname, def_realm);
}


On Mon, Nov 05, 2001 at 01:55:29PM -0500, Nicolas Williams wrote:
--

Visit our website at http://www.ubswarburg.com

This message contains confidential information and is intended only
for the individual named. If you are not the named addressee you
should not disseminate, distribute or copy this e-mail. Please
notify the sender immediately by e-mail if you have received this
e-mail by mistake and delete this e-mail from your system.

E-mail transmission cannot be guaranteed to be secure or error-free
as information could be intercepted, corrupted, lost, destroyed,
arrive late or incomplete, or contain viruses. The sender therefore
does not accept liability for any errors or omissions in the contents
of this message which arise as a result of e-mail transmission. If
verification is required please request a hard-copy version. This
message is provided for informational purposes and should not be
construed as a solicitation or offer to buy or sell any securities or
related financial instruments.