Bug 11024 - [patch] getpwnam(3) uses incorrect #define to limit username length
Summary: [patch] getpwnam(3) uses incorrect #define to limit username length
Status: Closed Overcome By Events
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 1999-04-08 14:40 UTC by rpb
Modified: 2014-10-02 07:15 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description rpb 1999-04-08 14:40:01 UTC
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
Comment 1 Mike Barcroft freebsd_committer 2001-07-20 02:58:27 UTC
State Changed
From-To: open->feedback


Does this problem still occur in newer versions of FreeBSD, 
such as 4.3-RELEASE?
Comment 2 Mike Barcroft freebsd_committer 2001-07-20 16:01:09 UTC
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.
Comment 3 Mike Barcroft freebsd_committer 2001-07-20 17:05:55 UTC
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.
Comment 4 Mike Barcroft freebsd_committer 2001-07-20 17:26:31 UTC
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/
> 
>
Comment 5 Mike Makonnen 2002-09-04 19:50:27 UTC
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
Comment 6 Mike Makonnen freebsd_committer 2003-01-08 10:45:28 UTC
State Changed
From-To: suspended->analyzed

As I indicated in a followup, I have a patch for this. 


Comment 7 Mike Makonnen freebsd_committer 2003-01-08 10:45:28 UTC
Responsible Changed
From-To: freebsd-bugs->mtm

I'll take this.
Comment 8 Mark Linimon freebsd_committer freebsd_triage 2013-07-03 01:50:32 UTC
State Changed
From-To: analyzed->analyzed

commit bit has been taken in for safekeeping. 


Comment 9 Mark Linimon freebsd_committer freebsd_triage 2013-07-03 01:50:32 UTC
Responsible Changed
From-To: mtm->freebsd-bugs
Comment 10 Baptiste Daroussin freebsd_committer 2014-10-02 07:15:17 UTC
UT_NAMESIZE is not used anymore