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 ; 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 ; 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 ; Mon, 5 Nov 2001 10:46:08 -0500 (EST) Received: from (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 ; 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 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 >Number: 1009 >Category: krb5-admin >Synopsis: kadmin_parse_name() loops forever with, e.g., '@SOME.REALM' >Confidential: no >Severity: non-critical >Priority: medium >Responsible: krb5-unassigned >State: open >Class: sw-bug >Submitter-Id: unknown >Arrival-Date: Mon Nov 5 10:45:00 EST 2001 >Last-Modified: Mon Nov 05 17:02:00 EST 2001 >Originator: Nicolas Williams >Organization: UBS Warburg / Perot Systems Corp. -- >Release: krb5-1.2.2 >Environment: Ultra-SPARC, Solaris 2.6 System: SunOS sm0d1989cmp 5.6 Generic_105181-12 sun4u sparc SUNW,Ultra-2 Architecture: sun4 >Description: 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 || ...). >How-To-Repeat: kadmin.local -q "ank -randkey @SWISSBANK.COM" >Fix: 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. >Audit-Trail: From: Nicolas Williams 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: > Thank you very much for your problem report. > It has the internal identification `krb5-admin/1009'. > The individual assigned to look at your > report is: krb5-unassigned. > > >Category: krb5-admin > >Responsible: krb5-unassigned > >Synopsis: kadmin_parse_name() loops forever with, e.g., '@SOME.REALM' > >Arrival-Date: Mon Nov 5 10:45:00 EST 2001 -- 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 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: > > 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: > > Thank you very much for your problem report. > > It has the internal identification `krb5-admin/1009'. > > The individual assigned to look at your > > report is: krb5-unassigned. > > > > >Category: krb5-admin > > >Responsible: krb5-unassigned > > >Synopsis: kadmin_parse_name() loops forever with, e.g., '@SOME.REALM' > > >Arrival-Date: Mon Nov 5 10:45:00 EST 2001 > -- -- 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. >Unformatted: