>Number: 187310
>Category: bin
>Synopsis: [patch] pw command segfaults when the -V parameter is used on
>commands that alter groups
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Mar 06 05:50:00 UTC 2014
>Closed-Date:
>Last-Modified:
>Originator: Kim Shrier
>Release: 10.0 Release amd64
>Organization:
>Environment:
FreeBSD snorri.lab.westryn.net 10.0-RELEASE FreeBSD 10.0-RELEASE #1 r261308M:
Sun Feb 23 18:45:19 MST 2014
ca...@snorri.lab.westryn.net:/usr/obj/usr/src/sys/SNORRI_02 amd64
>Description:
When specifying an alternate location of the etc directory using the -V command
line parameter to /usr/sbin/pw, the pw command segfaults when it does anything
that updates groups
>How-To-Repeat:
Create a master.passwd and group file in a directory other than /etc. For the
sake of this description, I'll use /tmp/pw_problem/etc.
mkdir -p /tmp/pw_problem/etc
cd /tmp/pw_problem/etc
Create a master.passwd file in this directory that contains a line like:
bob:*:1001:1001::0:0:Robert:/home/bob:/bin/sh
Create a group file in this directory that contains a line like:
bob:*:1001:
Run pwd_mkdb.
pwd_mkdb -p -d /tmp/pw_problem/etc master.passwd
Now, try to delete the user with pw
pw -V /tmp/pw_problem/etc userdel bob
Segmentation fault (core dumped)
>Fix:
The problem is that the gr_mem member of the group struct is dereferenced
without first checking to see if it is NULL. This occurs in both
/usr/src/usr.sbin/pw/pw_group.c and /usr/src/usr.sbin/pw/pw_user.c.
The reason this happens only when the -V parameter is used is because pw uses
different routines based on whether or not the -V parameter is present. When
it isn't specified, it uses getgrent, getgrgid, and getgrnam from libc. When
-V is specified, it uses vgetgrent, vgetgrgid, and vgetgrnam which uses code
from pw_vpw.c which is part of the source for pw. These three routines call
vnextgrent which eventually calls gr_scan from libutil. Looking at the source
in libutil, it is possible for the group structure returned by gr_scan to have
a NULL gr_mem. Other code in libutil deals with this possibility. The pw code
does not.
I am attaching a patch file that I made against head.
The rcsid for pw_group is:
static const char rcsid[] =
"$FreeBSD: head/usr.sbin/pw/pw_group.c 244738 2012-12-27 14:44:13Z bapt $";
The rcsid for pw_user is:
static const char rcsid[] =
"$FreeBSD: head/usr.sbin/pw/pw_user.c 252688 2013-07-04 07:59:11Z des $";
Patch attached with submission follows:
--- usr.sbin/pw/pw_group.c.orig 2014-03-05 21:12:10.000000000 -0700
+++ usr.sbin/pw/pw_group.c 2014-03-05 21:22:03.000000000 -0700
@@ -227,10 +227,12 @@
else if (arg->ch == 'm') {
int k = 0;
- while (grp->gr_mem[k] != NULL) {
- if (extendarray(&members, &grmembers, i + 2) !=
-1)
- members[i++] = grp->gr_mem[k];
- k++;
+ if (grp->gr_mem != NULL) {
+ while (grp->gr_mem[k] != NULL) {
+ if (extendarray(&members, &grmembers, i
+ 2) != -1)
+ members[i++] = grp->gr_mem[k];
+ k++;
+ }
}
}
@@ -311,6 +313,9 @@
int k;
struct passwd *pwd;
+ if (grp->gr_mem == NULL)
+ return;
+
k = 0;
while (grp->gr_mem[k] != NULL) {
matchFound = false;
@@ -415,8 +420,10 @@
printf("Group Name: %-15s #%lu\n"
" Members: ",
grp->gr_name, (long) grp->gr_gid);
- for (i = 0; grp->gr_mem[i]; i++)
- printf("%s%s", i ? "," : "", grp->gr_mem[i]);
+ if (grp->gr_mem != NULL) {
+ for (i = 0; grp->gr_mem[i]; i++)
+ printf("%s%s", i ? "," : "", grp->gr_mem[i]);
+ }
fputs("\n\n", stdout);
}
return EXIT_SUCCESS;
--- usr.sbin/pw/pw_user.c.orig 2014-03-05 21:12:10.000000000 -0700
+++ usr.sbin/pw/pw_user.c 2014-03-05 21:21:43.000000000 -0700
@@ -425,19 +425,21 @@
}
grp = GETGRNAM(a_name->val);
- if (grp != NULL && *grp->gr_mem == NULL)
+ if (grp != NULL && (grp->gr_mem == NULL || *grp->gr_mem
== NULL))
delgrent(GETGRNAM(a_name->val));
SETGRENT();
while ((grp = GETGRENT()) != NULL) {
int i;
char group[MAXLOGNAME];
- for (i = 0; grp->gr_mem[i] != NULL; i++) {
- if (!strcmp(grp->gr_mem[i],
a_name->val)) {
- while (grp->gr_mem[i] != NULL) {
- grp->gr_mem[i] =
grp->gr_mem[i+1];
- }
- strlcpy(group, grp->gr_name,
MAXLOGNAME);
- chggrent(group, grp);
+ if (grp->gr_mem != NULL) {
+ for (i = 0; grp->gr_mem[i] != NULL;
i++) {
+ if (!strcmp(grp->gr_mem[i],
a_name->val)) {
+ while (grp->gr_mem[i]
!= NULL) {
+ grp->gr_mem[i]
= grp->gr_mem[i+1];
+ }
+ strlcpy(group,
grp->gr_name, MAXLOGNAME);
+ chggrent(group, grp);
+ }
}
}
}
@@ -908,7 +910,7 @@
errx(EX_NOUSER, "group `%s' is not defined",
a_gid->val);
}
gid = grp->gr_gid;
- } else if ((grp = GETGRNAM(nam)) != NULL && grp->gr_mem[0] == NULL) {
+ } else if ((grp = GETGRNAM(nam)) != NULL && (grp->gr_mem == NULL ||
grp->gr_mem[0] == NULL)) {
gid = grp->gr_gid; /* Already created? Use it anyway... */
} else {
struct cargs grpargs;
@@ -1182,14 +1184,17 @@
while ((grp=GETGRENT()) != NULL)
{
int i = 0;
- while (grp->gr_mem[i] != NULL)
+ if (grp->gr_mem != NULL)
{
- if (strcmp(grp->gr_mem[i], pwd->pw_name)==0)
+ while (grp->gr_mem[i] != NULL)
{
- printf(j++ == 0 ? " Groups: %s" :
",%s", grp->gr_name);
- break;
+ if (strcmp(grp->gr_mem[i],
pwd->pw_name)==0)
+ {
+ printf(j++ == 0 ? " Groups:
%s" : ",%s", grp->gr_name);
+ break;
+ }
+ ++i;
}
- ++i;
}
}
ENDGRENT();
>Release-Note:
>Audit-Trail:
>Unformatted:
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
