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 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: 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)
Responsible Changed From-To: freebsd-bugs->julian Take this one. patch committed to -current revision 262864. Will MFC to 10,9,8 in a week
State Changed From-To: open->patched part 2 committed as revision 262865. MFC to come
Author: julian Date: Thu Mar 6 19:58:03 2014 New Revision: 262865 URL: http://svnweb.freebsd.org/changeset/base/262865 Log: Part 2 of bug 187310.. had to commit separately due to local confusion. Don't let pw crash when give certain input. PR: 187310 Submitted by: Kim Shrier MFC after: 1 week Modified: head/usr.sbin/pw/pw_user.c Modified: head/usr.sbin/pw/pw_user.c ============================================================================== --- head/usr.sbin/pw/pw_user.c Thu Mar 6 19:26:08 2014 (r262864) +++ head/usr.sbin/pw/pw_user.c Thu Mar 6 19:58:03 2014 (r262865) @@ -425,19 +425,22 @@ pw_user(struct userconf * cnf, int mode, } 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 +911,8 @@ pw_gidpolicy(struct userconf * cnf, stru 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 +1186,16 @@ print_user(struct passwd * pwd, int pret while ((grp=GETGRENT()) != NULL) { int i = 0; - while (grp->gr_mem[i] != NULL) - { - if (strcmp(grp->gr_mem[i], pwd->pw_name)==0) + if (grp->gr_mem != NULL) { + 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(); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: julian Date: Thu Mar 6 19:26:08 2014 New Revision: 262864 URL: http://svnweb.freebsd.org/changeset/base/262864 Log: Stop pw(8) from segfaulting when given certain input PR:187310 Submitted by: Kim Shrier Obtained from: bug MFC after: 1 week Modified: head/usr.sbin/pw/pw_group.c Modified: head/usr.sbin/pw/pw_group.c ============================================================================== --- head/usr.sbin/pw/pw_group.c Thu Mar 6 18:50:35 2014 (r262863) +++ head/usr.sbin/pw/pw_group.c Thu Mar 6 19:26:08 2014 (r262864) @@ -227,10 +227,12 @@ pw_group(struct userconf * cnf, int mode 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 @@ delete_members(char ***members, int *grm 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 @@ print_group(struct group * grp, int pret 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; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
The fixes haven't been committed to stable/9 yet, as noted in bug 169471 comment # 4. Reassigning to dteske for analysis. Please feel free to close this bug if you feel this commit shouldn't be MFCed to stable/9.
Will have a look; thanks guys.
batch change: For bugs that match the following - Status Is In progress AND - Untouched since 2018-01-01. AND - Affects Base System OR Documentation DO: Reset to open status. Note: I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
stable/9 is EOL; closing fixed