The getpwnam(3) library call compares against UT_NAMESIZE (8 in 2.x, 16 in 3.x). Shouldn't UT_NAMESIZE be used just to indicate the maximum characters stored in a UTMP entry, rather than to limit the allowable size of username? That's what #defines like 'MAXLOGNAME' are supposed to represent, and indeed many programs and library calls such as getlogin(3) use MAXLOGNAME in this way. Fix: modify getpwnam(3) et al to honour MAXLOGNAME - 1 instead of UT_NAMESIZE Also (3.1-stable) include/stdio.h lib/libc/gen/pwcache.c release/sysinstall/sysinstall/user.c usr.bin/chpass/pw_yp.c usr.bin/finger/finger.c (maybe?) usr.bin/login/login.c I've found about another 10 places where programs will produce unexpected output if UT_NAMESIZE isn't actually big enough to hold MAXLOGNAME - 1 characters, as was the case in FreeBSD 2.x
State Changed From-To: open->feedback Does this problem still occur in newer versions of FreeBSD, such as 4.3-RELEASE?
State Changed From-To: feedback->closed Originator indicated, in a private e-mail to me, that he isn't able to test this in a newer version of FreeBSD. But since the variable that was causing the problem no longer exists, I feel it's safe to close this PR.
State Changed From-To: closed->suspended Originator believes this is still a problem. I'll forward his e-mail to Gnats. Awaiting fix and committer.
Message from originator, that relates to this PR. On Fri, Jul 20, 2001 at 04:44:33PM +0100, Ray Bellis wrote: > > Synopsis: getpwnam(3) uses incorrect #define to limit username length > > > > State-Changed-From-To: feedback->closed > > State-Changed-By: mike > > State-Changed-When: Fri Jul 20 08:01:09 PDT 2001 > > State-Changed-Why: > > > > Originator indicated, in a private e-mail to me, that he isn't able to > > test this in a newer version of FreeBSD. But since the variable that > > was causing the problem no longer exists, I feel it's safe to close > > this PR. > > > > http://www.FreeBSD.org/cgi/query-pr.cgi?pr=11024 > > I disagree. > > Looking through the CVS web interfaces there are still places in the > source where UT_NAMESIZE is used instead of MAXLOGNAME (eg > usr.bin/login/login.c). If they happen to be the same value, it'll > still work, but if a sysadmin changes MAXLOGNAME it won't have the > desired effect. > > regards, > > Ray > > -- > Ray Bellis, MA(Oxon) - Technical Director > community internet plc - TicketingSolutions.com Ltd > > Windsor House, 12 High Street, Kidlington, Oxford, OX5 2PJ > tel: +44 1865 856000 email: ray.bellis@community.net.uk > fax: +44 1865 856001 web: http://www.community.net.uk/ > >
I tried to go through and fix all the places UT_NAMESIZE is used instead of MAXLOGNAME. I did not touch any program that uses the w/utmp files, contributed software (except for top), or posix files in libc. Cheers, Mike Makonnen Index: bin/pax/gen_subs.c =================================================================== RCS file: /home/ncvs/src/bin/pax/gen_subs.c,v retrieving revision 1.20 diff -u -r1.20 gen_subs.c --- bin/pax/gen_subs.c 30 Jun 2002 05:15:01 -0000 1.20 +++ bin/pax/gen_subs.c 3 Sep 2002 22:24:01 -0000 @@ -43,12 +43,12 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD: src/bin/pax/gen_subs.c,v 1.20 2002/06/30 05:15:01 obrien Exp $"); +#include <sys/param.h> #include <sys/types.h> #include <sys/time.h> #include <sys/stat.h> #include <langinfo.h> #include <stdio.h> -#include <utmp.h> #include <unistd.h> #include <stdlib.h> #include <string.h> @@ -69,8 +69,8 @@ #define OLDFRMTM "%b %e %Y" #define CURFRMTD "%e %b %H:%M" #define OLDFRMTD "%e %b %Y" -#ifndef UT_NAMESIZE -#define UT_NAMESIZE 8 +#ifndef MAXLOGNAME +#define MAXLOGNAME 8 #endif #define UT_GRPSIZE 6 @@ -120,7 +120,7 @@ if (strftime(f_date,DATELEN,timefrmt,localtime(&(sbp->st_mtime))) == 0) f_date[0] = '\0'; (void)fprintf(fp, "%s%2u %-*s %-*s ", f_mode, sbp->st_nlink, - UT_NAMESIZE, name_uid(sbp->st_uid, 1), UT_GRPSIZE, + MAXLOGNAME, name_uid(sbp->st_uid, 1), UT_GRPSIZE, name_gid(sbp->st_gid, 1)); /* Index: contrib/top/username.c =================================================================== RCS file: /home/ncvs/src/contrib/top/username.c,v retrieving revision 1.3 diff -u -r1.3 username.c --- contrib/top/username.c 9 Jun 2002 19:29:55 -0000 1.3 +++ contrib/top/username.c 3 Sep 2002 22:41:58 -0000 @@ -30,17 +30,17 @@ * This makes the table size independent of the passwd file size. */ +#include <sys/param.h> #include <sys/types.h> #include <stdio.h> #include <pwd.h> -#include <utmp.h> #include "top.local.h" #include "utils.h" struct hash_el { int uid; - char name[UT_NAMESIZE + 1]; + char name[MAXLOGNAME]; /* MAXLOGNAME includes space for the NULL */ }; #define is_empty_hash(x) (hash_table[x].name[0] == 0) @@ -129,7 +129,7 @@ /* empty or wrong slot -- fill it with new value */ hash_table[hashindex].uid = uid; - (void) strncpy(hash_table[hashindex].name, name, UT_NAMESIZE); + (void) strncpy(hash_table[hashindex].name, name, MAXLOGNAME - 1); return(hashindex); } Index: lib/libc/gen/pwcache.c =================================================================== RCS file: /home/ncvs/src/lib/libc/gen/pwcache.c,v retrieving revision 1.10 diff -u -r1.10 pwcache.c --- lib/libc/gen/pwcache.c 22 Mar 2002 02:35:47 -0000 1.10 +++ lib/libc/gen/pwcache.c 3 Sep 2002 22:52:17 -0000 @@ -37,13 +37,13 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD: src/lib/libc/gen/pwcache.c,v 1.10 2002/03/22 02:35:47 imp Exp $"); +#include <sys/param.h> #include <sys/types.h> #include <grp.h> #include <pwd.h> #include <stdio.h> #include <string.h> -#include <utmp.h> #define NCACHE 64 /* power of 2 */ #define MASK (NCACHE - 1) /* bits to store with */ @@ -54,7 +54,7 @@ static struct ncache { uid_t uid; int found; - char name[UT_NAMESIZE + 1]; + char name[MAXLOGNAME]; /* Includes terminating NULL */ } c_uid[NCACHE]; static int pwopen; struct passwd *pw; @@ -70,11 +70,11 @@ cp->uid = uid; if (pw != NULL) { cp->found = 1; - (void)strncpy(cp->name, pw->pw_name, UT_NAMESIZE); - cp->name[UT_NAMESIZE] = '\0'; + (void)strncpy(cp->name, pw->pw_name, MAXLOGNAME - 1); + cp->name[MAXLOGNAME - 1] = '\0'; } else { cp->found = 0; - (void)snprintf(cp->name, UT_NAMESIZE, "%u", uid); + (void)snprintf(cp->name, MAXLOGNAME - 1, "%u", uid); if (nouser) return (NULL); } @@ -88,7 +88,7 @@ static struct ncache { gid_t gid; int found; - char name[UT_NAMESIZE + 1]; + char name[MAXLOGNAME]; /* Includes terminating NULL */ } c_gid[NCACHE]; static int gropen; struct group *gr; @@ -104,11 +104,11 @@ cp->gid = gid; if (gr != NULL) { cp->found = 1; - (void)strncpy(cp->name, gr->gr_name, UT_NAMESIZE); - cp->name[UT_NAMESIZE] = '\0'; + (void)strncpy(cp->name, gr->gr_name, MAXLOGNAME - 1); + cp->name[MAXLOGNAME - 1] = '\0'; } else { cp->found = 0; - (void)snprintf(cp->name, UT_NAMESIZE, "%u", gid); + (void)snprintf(cp->name, MAXLOGNAME - 1, "%u", gid); if (nogroup) return (NULL); } Index: usr.bin/lastcomm/lastcomm.c =================================================================== RCS file: /home/ncvs/src/usr.bin/lastcomm/lastcomm.c,v retrieving revision 1.16 diff -u -r1.16 lastcomm.c --- usr.bin/lastcomm/lastcomm.c 30 Jun 2002 05:25:03 -0000 1.16 +++ usr.bin/lastcomm/lastcomm.c 3 Sep 2002 23:13:26 -0000 @@ -179,7 +179,7 @@ (void)printf("%-*.*s %-7s %-*s %-*s", AC_COMM_LEN, AC_COMM_LEN, ab.ac_comm, flagbits(ab.ac_flag), - UT_NAMESIZE, user_from_uid(ab.ac_uid, 0), + MAXLOGNAME - 1, user_from_uid(ab.ac_uid, 0), UT_LINESIZE, getdev(ab.ac_tty)); Index: usr.sbin/repquota/repquota.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/repquota/repquota.c,v retrieving revision 1.14 diff -u -r1.14 repquota.c --- usr.sbin/repquota/repquota.c 11 Jul 2002 21:24:49 -0000 1.14 +++ usr.sbin/repquota/repquota.c 3 Sep 2002 23:28:49 -0000 @@ -63,7 +63,6 @@ #include <string.h> #include <time.h> #include <unistd.h> -#include <utmp.h> /* Let's be paranoid about block size */ #if 10 > DEV_BSHIFT @@ -229,9 +228,9 @@ } fclose(qf); printf("%*s Block limits File limits\n", - max(UT_NAMESIZE,10), " "); + max(MAXLOGNAME - 1,10), " "); printf("User%*s used soft hard grace used soft hard grace\n", - max(UT_NAMESIZE,10), " "); + max(MAXLOGNAME - 1,10), " "); for (id = 0; id <= highid[type]; id++) { fup = lookup(id, type); if (fup == 0) @@ -239,7 +238,7 @@ if (fup->fu_dqblk.dqb_curinodes == 0 && fup->fu_dqblk.dqb_curblocks == 0) continue; - printf("%-*s", max(UT_NAMESIZE,10), fup->fu_name); + printf("%-*s", max(MAXLOGNAME - 1,10), fup->fu_name); printf("%c%c %8lu %8lu %8lu %6s", fup->fu_dqblk.dqb_bsoftlimit && fup->fu_dqblk.dqb_curblocks >= Index: usr.sbin/sysinstall/user.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/sysinstall/user.c,v retrieving revision 1.17 diff -u -r1.17 user.c --- usr.sbin/sysinstall/user.c 5 Jul 2001 09:51:09 -0000 1.17 +++ usr.sbin/sysinstall/user.c 3 Sep 2002 23:40:23 -0000 @@ -35,7 +35,6 @@ */ #include "sysinstall.h" -#include <utmp.h> #include <ctype.h> #include <sys/param.h> #include <sysexits.h> @@ -61,7 +60,7 @@ static char gname[GNAME_FIELD_LEN], gid[GID_FIELD_LEN], gmemb[GMEMB_FIELD_LEN], - uname[UT_NAMESIZE + 1], + uname[MAXLOGNAME], passwd[PASSWD_FIELD_LEN], uid[UID_FIELD_LEN], ugroup[UGROUP_FIELD_LEN], @@ -108,7 +107,7 @@ /* The user configuration menu. */ static Layout userLayout[] = { #define LAYOUT_UNAME 0 - { 3, 6, UT_NAMESIZE, UT_NAMESIZE + 1, + { 3, 6, MAXLOGNAME - 1, MAXLOGNAME, "Login ID:", "The login name of the new user (mandatory)", uname, STRINGOBJ, NULL }, #define LAYOUT_UID 1
State Changed From-To: suspended->analyzed As I indicated in a followup, I have a patch for this.
Responsible Changed From-To: freebsd-bugs->mtm I'll take this.
State Changed From-To: analyzed->analyzed commit bit has been taken in for safekeeping.
Responsible Changed From-To: mtm->freebsd-bugs
UT_NAMESIZE is not used anymore