NetBSD Problem Report #13974
Received: (qmail 22851 invoked from network); 16 Sep 2001 17:56:11 -0000
Message-Id: <200109161756.f8GHuCM16017@miyu.feyrer.net>
Date: Sun, 16 Sep 2001 19:56:12 +0200 (MEST)
From: Hubert Feyrer <hubert@feyrer.de>
Reply-To: hubert.feyrer@rz.uni-regensburg.de
To: gnats-bugs@gnats.netbsd.org
Subject: usermod/add etc. group handling problems
X-Send-Pr-Version: 3.95
>Number: 13974
>Category: bin
>Synopsis: usermod/add etc. group handling problems
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun Sep 16 17:57:01 +0000 2001
>Closed-Date:
>Last-Modified: Thu Jan 24 16:13:41 +0000 2008
>Originator: Hubert Feyrer
>Release: NetBSD 4.0, originally submitted for NetBSD 1.5.1
>Organization:
bla!
>Environment:
System:
* NetBSD 4.0/i386
* NetBSD miyu 1.5.2 NetBSD 1.5.2 (MIYU) #13: Tue Sep 11 22:00:05 MEST 2001 feyrer@miyu:/usr/cvs/src-1.5/sys/arch/i386/compile/MIYU i386
>Description:
Looking at handling setups with many users as e.g. found on
cvs.netbsd.org, I found several problems with useradd/usermod
WRT group handling.
1. When adding users, the max. 1000 line limit in our
routines is not taken care of. Bad thing is if a line in
/etc/group grows beyond 1000 bytes, NONE of the users
will belong to that group any more.
2. When adding a user to a group manually (usermod -G), and
/etc/group has the same group more than once (to work around
the afore-mentioned 1000 bytes limit), the user is added
to to each line
3. When removing a user (userdel), he is left in /etc/group.
>How-To-Repeat:
0. Add "test2" group: groupadd test2
1. * Create a bunch of accounts:
perl -e 'for($i=0;$i<200;$i++){printf "useradd test%03d; \
usermod -d /tmp -G test2 test%03d\n", $i, $i;}' | sh -v
* Change to last user: su - test200
* Run "id" to see the user is not in the "test2" group
* exit back to r00t
* Change to first user: su - test000
* Run "id" to see the user is not in the "test2" group either
* grep ^test2: /etc/group | wc -c, and see the line is longer
than 1000 bytes
* Split it into two lines ("test2:*:111:test000,...,test100",
"test2:*:111:test101,...,test200") and see the above "su"
commands show the user being member of "test2"
2. * Add a new user: useradd -d /tmp test201
* Add the user to our (now-split) test2-group: usermod -G test2 test201
* grep ^test2: /etc/group and note that test201 is now on ALL
lines of test2 groups!
* Play with "useradd -G", and notice the same problem
3. * Remove all users:
perl -e 'for($i=0;$i<200;$i++){printf "userdel test%03d\n", \
$i;}' | sh -v
* Notice /etc/group still lists all users as part of the
test2 group
>Fix:
Unknown.
Need to read all users of each group into some list, and when
writing that out pay attention to the line limit.
As an alternative, fix our group handling (but that's probably
not gonna happen), so fixing user* is the easier thing here.
>Release-Note:
>Audit-Trail:
From: "Arne H. Juul" <arnej@pvv.ntnu.no>
To: gnats-bugs@gnats.netbsd.org
Cc: Hubert Feyrer <hubert@feyrer.de>, Alistair Crooks <agc@NetBSD.org>
Subject: Re: bin/13974
Date: Sun, 14 Mar 2004 13:25:28 +0100 (CET)
Here is a patch that fixes several problems related
to long group lines; this one auto-splits long groups
when adding a user would overflow the line limit.
It also detects if any old lines were too long and just exits
instead of possibly making things worse, like it would before.
Line limits are still hardcoded but now
to the same values as getgrent and getpwent have
hard-coded in them.
I've also made it detect when adding some secondary
groups for a user would overflow NGROUPS_MAX, and
fixed a couple of bugs related to this.
Index: usr.sbin/user/user.c
===================================================================
RCS file: /usr/cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.75
diff -u -r1.75 user.c
--- usr.sbin/user/user.c 14 Jan 2004 09:35:33 -0000 1.75
+++ usr.sbin/user/user.c 14 Mar 2004 13:09:21 -0000
@@ -167,8 +167,8 @@
MaxUserNameLen = 32,
MaxFieldNameLen = 32,
MaxCommandLen = 2048,
- MaxEntryLen = 2048,
- PasswordLength = 2048,
+ MaxEntryLen = 1024,
+ PasswordLength = 1024,
DES_Len = 13,
@@ -473,7 +473,6 @@
FILE *to;
char buf[MaxEntryLen];
char f[MaxFileNameLen];
- char *colon;
int groupc;
int entc;
int fd;
@@ -502,18 +501,42 @@
}
groupc = strlen(group);
while (fgets(buf, sizeof(buf), from) != NULL) {
+ char *firstcolon;
+ char *thirdcolon;
+
cc = strlen(buf);
- if ((colon = strchr(buf, ':')) == NULL) {
- warn("badly formed entry `%s'", buf);
- continue;
+ /*
+ * group entries are name : pwd : gid : members,
+ * so look for three colons
+ */
+ thirdcolon = firstcolon = strchr(buf, ':');
+ if (thirdcolon != NULL) thirdcolon = strchr(thirdcolon+1, ':');
+ if (thirdcolon != NULL) thirdcolon = strchr(thirdcolon+1, ':');
+
+ if (thirdcolon == NULL || buf[cc-1] != '\n') {
+ warnx("badly formed entry `%s'", buf);
+ (void) fclose(from);
+ (void) close(fd);
+ (void) unlink(f);
+ return 0;
}
- entc = (int)(colon - buf);
+ entc = (int)(firstcolon - buf);
if (entc == groupc && strncmp(group, buf, (unsigned) entc) == 0) {
if (newent == NULL) {
continue;
} else {
- cc = strlen(newent);
- (void) strlcpy(buf, newent, sizeof(buf));
+ char buf2[2*MaxEntryLen];
+
+ (void)strlcpy(buf2, newent, sizeof(buf2));
+ (void)strlcat(buf2, thirdcolon+1, sizeof(buf2));
+ cc = strlcpy(buf, buf2, sizeof(buf));
+ if (cc >= sizeof(buf)) {
+ (void) fclose(from);
+ (void) close(fd);
+ (void) unlink(f);
+ warnx("too long group entry '%s'", buf2);
+ return 0;
+ }
}
}
if (fwrite(buf, sizeof(char), (unsigned) cc, to) != cc) {
@@ -542,7 +565,7 @@
/* modify the group entries for all `groups', by adding `user' */
static int
-append_group(char *user, int ngroups, const char **groups)
+append_group(char *user, gid_t gid, int ngroups, const char **groups)
{
struct group *grp;
struct stat st;
@@ -558,19 +581,40 @@
int cc;
int i;
int j;
+ gid_t oldgrouplist[NGROUPS_MAX];
+ int numoldg = NGROUPS_MAX;
+
+ nc = getgrouplist(user, gid, oldgrouplist, &numoldg);
+ if (nc < 0) {
+ if (numoldg > NGROUPS_MAX)
+ warnx("user in %d groups already, max is %d",
+ numoldg, NGROUPS_MAX);
+ else
+ warn("can't get grouplist for user `%s'", user);
+ return 0;
+ }
+ /* stupid n*m algorithm is probably OK */
for (i = 0 ; i < ngroups ; i++) {
- if ((grp = getgrnam(groups[i])) == NULL) {
- warnx("can't append group `%s' for user `%s'", groups[i], user);
- } else {
- for (j = 0 ; grp->gr_mem[j] ; j++) {
- if (strcmp(user, grp->gr_mem[j]) == 0) {
- /* already in it */
- groups[i] = "";
- }
+ for (j = 0; j < numoldg; j++) {
+ grp = getgrgid(oldgrouplist[j]);
+ if (grp == NULL) {
+ warn("can't getgrgid `%d'", oldgrouplist[j]);
+ }
+ if (strcmp(groups[i], grp->gr_name) == 0) {
+ /* already in it */
+ oldgrouplist[j] = oldgrouplist[--numoldg];
+ groups[i] = "";
+ break;
}
}
}
+ if (ngroups + numoldg > NGROUPS_MAX) {
+ warnx("can't be member of %d groups (max is %d)",
+ ngroups + numoldg, NGROUPS_MAX);
+ return 0;
+ }
+
if ((from = fopen(_PATH_GROUP, "r")) == NULL) {
warn("can't append group for `%s': can't open `%s'", user, _PATH_GROUP);
return 0;
@@ -594,9 +638,13 @@
}
while (fgets(buf, sizeof(buf), from) != NULL) {
cc = strlen(buf);
- if ((colon = strchr(buf, ':')) == NULL) {
- warn("badly formed entry `%s'", buf);
- continue;
+ if ((colon = strchr(buf, ':')) == NULL || buf[cc-1] != '\n') {
+ warnx("Error: badly formed group entry `%s'", buf);
+ /* it's not safe to continue */
+ (void) fclose(from);
+ (void) close(fd);
+ (void) unlink(f);
+ return 0;
}
entc = (int)(colon - buf);
for (i = 0 ; i < ngroups ; i++) {
@@ -604,14 +652,38 @@
continue;
}
if (entc == groupc && strncmp(groups[i], buf, (unsigned) entc) == 0) {
- if ((nc = snprintf(&buf[cc - 1],
- sizeof(buf) - cc + 1,
- "%s%s\n",
- (buf[cc - 2] == ':') ? "" : ",",
- user)) < 0) {
- warnx("Warning: group `%s' entry too long", groups[i]);
+ /* need space for old group entry, comma, username, newline */
+ if (cc + strlen(user) + 2 < sizeof(buf)) {
+ nc = snprintf(&buf[cc - 1],
+ sizeof(buf) - (cc - 1),
+ "%s%s\n",
+ (buf[cc - 2] == ':') ? "" : ",",
+ user);
+ if (nc < 0 || nc > sizeof(buf) - (cc-1)) {
+ /* this should not happen */
+ errx(EXIT_FAILURE, "Error: snprintf failed on group `%s'", groups[i]);
+ }
+ cc += nc - 1;
+ } else {
+ char buf2[MaxEntryLen];
+ warnx("Warning: group `%s' entry too long, splitting", groups[i]);
+ colon = strrchr(buf, ':');
+ nc = snprintf(buf2, sizeof(buf2), "%.*s:%s\n", colon - buf, buf, user);
+ if (nc < 0 || nc > sizeof(buf2)) {
+ warnx("Error: cannot even make split group `%s' entry", buf2);
+ } else {
+ warnx("new entry: '%s'", buf2);
+ if (fwrite(buf2, sizeof(char), (unsigned) nc, to) != nc) {
+ (void) fclose(from);
+ (void) close(fd);
+ (void) unlink(f);
+ warn("can't create gid: short write to `%s'", f);
+ return 0;
+ }
+ }
}
- cc += nc - 1;
+ /* done this once now, avoid doing it again */
+ groups[i] = "";
}
}
if (fwrite(buf, sizeof(char), (unsigned) cc, to) != cc) {
@@ -1118,7 +1190,7 @@
pw_abort();
errx(EXIT_FAILURE, "can't create gid %d for login name %s", gid, login_name);
}
- if (up->u_groupc > 0 && !append_group(login_name, up->u_groupc, up->u_groupv)) {
+ if (up->u_groupc > 0 && !append_group(login_name, gid, up->u_groupc, up->u_groupv)) {
(void) close(ptmpfd);
pw_abort();
errx(EXIT_FAILURE, "can't append `%s' to new groups", login_name);
@@ -1425,7 +1497,7 @@
homedir, pwp->pw_dir);
}
if (up->u_groupc > 0 &&
- !append_group(newlogin, up->u_groupc, up->u_groupv)) {
+ !append_group(newlogin, pwp->pw_gid, up->u_groupc, up->u_groupv)) {
(void) close(ptmpfd);
pw_abort();
errx(EXIT_FAILURE, "can't append `%s' to new groups",
@@ -1566,8 +1638,9 @@
bigD = 1;
break;
case 'G':
- while ((u.u_groupv[u.u_groupc] = strsep(&optarg, ",")) != NULL &&
- u.u_groupc < NGROUPS_MAX) {
+ while (u.u_groupc < NGROUPS_MAX &&
+ (u.u_groupv[u.u_groupc] = strsep(&optarg, ",")) != NULL)
+ {
if (u.u_groupv[u.u_groupc][0] != 0) {
u.u_groupc++;
}
@@ -1702,8 +1775,9 @@
while ((c = getopt(argc, argv, "G:c:d:e:f:g:l:mos:u:" MOD_OPT_EXTENSIONS)) != -1) {
switch(c) {
case 'G':
- while ((u.u_groupv[u.u_groupc] = strsep(&optarg, ",")) != NULL &&
- u.u_groupc < NGROUPS_MAX) {
+ while (u.u_groupc < NGROUPS_MAX &&
+ (u.u_groupv[u.u_groupc] = strsep(&optarg, ",")) != NULL)
+ {
if (u.u_groupv[u.u_groupc][0] != 0) {
u.u_groupc++;
}
@@ -1998,7 +2072,6 @@
struct group *grp;
char buf[MaxEntryLen];
char *newname;
- char **cpp;
int dupgid;
int gid;
int cc;
@@ -2056,11 +2129,10 @@
(newname) ? newname : grp->gr_name,
grp->gr_passwd,
(gid < 0) ? grp->gr_gid : gid);
- for (cpp = grp->gr_mem ; *cpp && cc < sizeof(buf) ; cpp++) {
- cc += snprintf(&buf[cc], sizeof(buf) - cc, "%s%s", *cpp,
- (cpp[1] == NULL) ? "" : ",");
+ /* we need some space for members */
+ if (2*cc > sizeof(buf)) {
+ errx(EXIT_FAILURE, "group entry '%s' too long", buf);
}
- cc += snprintf(&buf[cc], sizeof(buf) - cc, "\n");
openlog("groupmod", LOG_PID, LOG_USER);
if (!modify_gid(*argv, buf)) {
err(EXIT_FAILURE, "can't change %s file", _PATH_GROUP);
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.