MFC: 272445,272578,273772,273779,273782,273786,273787,273791

Add a test for bug 191427 where pw(8) will go into an infinite loop
Add some tests for modifying groups
When a group is renamed then the group has been invalidated for sure.
In that case get the group information using the new name.

Fix a regression in pw usermod -G list

The user was perperly adding the to different groups from "list" but was not
removed from the other groups it could have belong to.

Do not delete the group wheel when bad argument is passed to pw groupdel -g

Check that the -g argument is actually a number, if not report an error.
This argument is converted without checking with atoi(3) later so without this
check it converts any alpha entries into 0 meaning it deletes the group wheel

Ensure pw userdel -u <invalid> do not try to remove root

Check the uid passed is actually a number as early as possible

Fix renaming a group via the gr_copy function

Add a regression test to pw(8) because the bug was discovered via using:
pw groupmod

PR:		193704 [1], 185666 [2], 90114 [3], 187189 [4]
Submitted by:	Marc de la Gueronniere [4]
Reported by:	az [1], sub.mesa@gmail.com [2], bkoenig@cs.tu-berlin.de [3],
		mcdouga9@egr.msu.edu [4]

11 files changed, 213 insertions, 10 deletions

diff --git a/etc/mtree/BSD.tests.dist b/etc/mtree/BSD.tests.dist
index 37f3c26..80e537a 100644
--- a/etc/mtree/BSD.tests.dist
+++ b/etc/mtree/BSD.tests.dist
@@ -273,6 +273,8 @@
             ..
             newsyslog
             ..
+            pw
+            ..
             sa
             ..
         ..
diff --git a/lib/libutil/gr_util.c b/lib/libutil/gr_util.c
index 6f74507..465efd9 100644
--- a/lib/libutil/gr_util.c
+++ b/lib/libutil/gr_util.c
@@ -170,14 +170,21 @@ gr_copy(int ffd, int tfd, const struct group *gr, struct group *old_gr)
 	size_t len;
 	int eof, readlen;
 
-	sgr = gr;
+	if (old_gr == NULL && gr == NULL)
+		return(-1);
+
+	sgr = old_gr;
+	/* deleting a group */
 	if (gr == NULL) {
 		line = NULL;
-		if (old_gr == NULL)
+	} else {
+		if ((line = gr_make(gr)) == NULL)
 			return (-1);
-		sgr = old_gr;
-	} else if ((line = gr_make(gr)) == NULL)
-		return (-1);
+	}
+
+	/* adding a group */
+	if (sgr == NULL)
+		sgr = gr;
 
 	eof = 0;
 	len = 0;
diff --git a/usr.sbin/pw/Makefile b/usr.sbin/pw/Makefile
index eae0b87..b3c8a39 100644
--- a/usr.sbin/pw/Makefile
+++ b/usr.sbin/pw/Makefile
@@ -1,14 +1,20 @@
 # $FreeBSD$
 
+.include <bsd.own.mk>
+
 PROG=	pw
 MAN=	pw.conf.5 pw.8
 SRCS=	pw.c pw_conf.c pw_user.c pw_group.c pw_log.c pw_nis.c pw_vpw.c \
 	grupd.c pwupd.c fileupd.c psdate.c \
 	bitmap.c cpdir.c rm_r.c
 
-WARNS?=	2
+WARNS?=	3
 
 DPADD=	${LIBCRYPT} ${LIBUTIL}
 LDADD=	-lcrypt -lutil
 
+.if ${MK_TESTS} != "no"
+SUBDIR+=	tests
+.endif
+
 .include <bsd.prog.mk>
diff --git a/usr.sbin/pw/pw_group.c b/usr.sbin/pw/pw_group.c
index 391e477..b20ce88 100644
--- a/usr.sbin/pw/pw_group.c
+++ b/usr.sbin/pw/pw_group.c
@@ -51,6 +51,7 @@ int
 pw_group(struct userconf * cnf, int mode, struct cargs * args)
 {
 	int		rc;
+	struct carg    *a_newname = getarg(args, 'l');
 	struct carg    *a_name = getarg(args, 'n');
 	struct carg    *a_gid = getarg(args, 'g');
 	struct carg    *arg;
@@ -66,6 +67,11 @@ pw_group(struct userconf * cnf, int mode, struct cargs * args)
 		NULL
 	};
 
+	if (a_gid != NULL) {
+		if (strspn(a_gid->val, "0123456789") != strlen(a_gid->val))
+			errx(EX_USAGE, "-g expects a number");
+	}
+
 	if (mode == M_LOCK || mode == M_UNLOCK)
 		errx(EX_USAGE, "'lock' command is not available for groups");
 
@@ -140,8 +146,8 @@ pw_group(struct userconf * cnf, int mode, struct cargs * args)
 		if (a_gid)
 			grp->gr_gid = (gid_t) atoi(a_gid->val);
 
-		if ((arg = getarg(args, 'l')) != NULL)
-			grp->gr_name = pw_checkname((u_char *)arg->val, 0);
+		if (a_newname != NULL)
+			grp->gr_name = pw_checkname((u_char *)a_newname->val, 0);
 	} else {
 		if (a_name == NULL)	/* Required */
 			errx(EX_DATAERR, "group name required");
@@ -270,8 +276,10 @@ pw_group(struct userconf * cnf, int mode, struct cargs * args)
 			warn("group update");
 		return EX_IOERR;
 	}
+
+	arg = a_newname != NULL ? a_newname : a_name;
 	/* grp may have been invalidated */
-	if ((grp = GETGRNAM(a_name->val)) == NULL)
+	if ((grp = GETGRNAM(arg->val)) == NULL)
 		errx(EX_SOFTWARE, "group disappeared during update");
 
 	pw_log(cnf, mode, W_GROUP, "%s(%ld)", grp->gr_name, (long) grp->gr_gid);
diff --git a/usr.sbin/pw/pw_user.c b/usr.sbin/pw/pw_user.c
index efb2901..483148a 100644
--- a/usr.sbin/pw/pw_user.c
+++ b/usr.sbin/pw/pw_user.c
@@ -321,6 +321,9 @@ pw_user(struct userconf * cnf, int mode, struct cargs * args)
 			(a_uid = a_name)->ch = 'u';
 			a_name = NULL;
 		}
+	} else {
+		if (strspn(a_uid->val, "0123456789") != strlen(a_uid->val))
+			errx(EX_USAGE, "-u expects a number");
 	}
 
 	/*
@@ -751,7 +754,25 @@ pw_user(struct userconf * cnf, int mode, struct cargs * args)
 	 */
 
 	if (mode == M_ADD || getarg(args, 'G') != NULL) {
-		int i;
+		int i, j;
+		/* First remove the user from all group */
+		SETGRENT();
+		while ((grp = GETGRENT()) != NULL) {
+			char group[MAXLOGNAME];
+			if (grp->gr_mem == NULL)
+				continue;
+			for (i = 0; grp->gr_mem[i] != NULL; i++) {
+				if (strcmp(grp->gr_mem[i] , pwd->pw_name) != 0)
+					continue;
+				for (j = i; grp->gr_mem[j] != NULL ; j++)
+					grp->gr_mem[j] = grp->gr_mem[j+1];
+				strlcpy(group, grp->gr_name, MAXLOGNAME);
+				chggrent(group, grp);
+			}
+		}
+		ENDGRENT();
+
+		/* now add to group where needed */
 		for (i = 0; cnf->groups[i] != NULL; i++) {
 			grp = GETGRNAM(cnf->groups[i]);
 			grp = gr_add(grp, pwd->pw_name);
diff --git a/usr.sbin/pw/tests/Makefile b/usr.sbin/pw/tests/Makefile
new file mode 100644
index 0000000..3003c8f
--- /dev/null
+++ b/usr.sbin/pw/tests/Makefile
@@ -0,0 +1,10 @@
+# $FreeBSD$
+
+TESTSDIR=	${TESTSBASE}/usr.sbin/pw
+
+ATF_TESTS_SH=	pw_delete pw_modify
+
+FILES=		group helper_functions.shin master.passwd
+FILESDIR=       ${TESTSDIR}
+
+.include <bsd.test.mk>
diff --git a/usr.sbin/pw/tests/group b/usr.sbin/pw/tests/group
new file mode 100644
index 0000000..620c588
--- /dev/null
+++ b/usr.sbin/pw/tests/group
@@ -0,0 +1,3 @@
+# $FreeBSD$
+#
+wheel:*:0:root
diff --git a/usr.sbin/pw/tests/helper_functions.shin b/usr.sbin/pw/tests/helper_functions.shin
new file mode 100755
index 0000000..f87b1e7
--- /dev/null
+++ b/usr.sbin/pw/tests/helper_functions.shin
@@ -0,0 +1,15 @@
+# $FreeBSD$
+
+# Workdir to run tests in
+TESTDIR=$(atf_get_srcdir)
+
+# Populate the files pw needs to use into $HOME/etc
+populate_etc_skel() {
+	cp ${TESTDIR}/master.passwd ${HOME} || \
+		atf_fail "Populating master.passwd in ${HOME}"
+	cp ${TESTDIR}/group ${HOME} || atf_fail "Populating group in ${HOME}"
+
+	# Generate the passwd file
+	pwd_mkdb -p -d ${HOME} ${HOME}/master.passwd || \
+		atf_fail "generate passwd from master.passwd"
+}
diff --git a/usr.sbin/pw/tests/master.passwd b/usr.sbin/pw/tests/master.passwd
new file mode 100644
index 0000000..f7dc837
--- /dev/null
+++ b/usr.sbin/pw/tests/master.passwd
@@ -0,0 +1,4 @@
+# $FreeBSD$
+#
+root:*:0:0::0:0:Charlie &:/root:/bin/csh
+toor:*:0:0::0:0:Bourne-again Superuser:/root:
diff --git a/usr.sbin/pw/tests/pw_delete.sh b/usr.sbin/pw/tests/pw_delete.sh
new file mode 100755
index 0000000..0e97939
--- /dev/null
+++ b/usr.sbin/pw/tests/pw_delete.sh
@@ -0,0 +1,47 @@
+# $FreeBSD$
+
+# Import helper functions
+. $(atf_get_srcdir)/helper_functions.shin
+
+# Test that a user can be deleted when another user is part of this
+# user's default group and does not go into an infinate loop.
+# PR: 191427
+atf_test_case rmuser_seperate_group cleanup
+rmuser_seperate_group_head() {
+	atf_set "timeout" "30"
+}
+rmuser_seperate_group_body() {
+	populate_etc_skel
+	pw -V ${HOME} useradd test || atf_fail "Creating test user"
+	pw -V ${HOME} groupmod test -M 'test,root' || \
+		atf_fail "Modifying the group"
+	pw -V ${HOME} userdel test || atf_fail "delete the user"
+}
+
+atf_test_case group_do_not_delete_wheel_if_group_unkown
+group_do_not_delete_wheel_if_group_unkown_head() {
+	atf_set "descr" "Make sure we do not consider as gid 0 an unknown group"
+}
+
+group_do_not_delete_wheel_if_group_unkown_body() {
+	populate_etc_skel
+	atf_check -s exit:0 -o inline:"wheel:*:0:root\n" -x pw -V ${HOME} groupshow wheel
+	atf_check -e inline:"pw: -g expects a number\n" -s exit:64 -x pw -V ${HOME} groupdel -g I_do_not_exist
+	atf_check -s exit:0 -o inline:"wheel:*:0:root\n" -x pw -V ${HOME} groupshow wheel
+}
+
+atf_test_case user_do_not_try_to_delete_root_if_user_unkown
+user_do_not_try_to_delete_root_if_user_unkown_head() {
+	atf_set "descr" "Make sure not to try to remove root if deleteing an unknown user"
+}
+
+user_do_not_try_to_delete_root_if_user_unkown_body() {
+	populate_etc_skel
+	atf_check -e inline:"pw: -u expects a number\n" -s exit:64 -x pw -V ${HOME} userdel -u plop
+}
+
+atf_init_test_cases() {
+	atf_add_test_case rmuser_seperate_group
+	atf_add_test_case group_do_not_delete_wheel_if_group_unkown
+	atf_add_test_case user_do_not_try_to_delete_root_if_user_unkown
+}
diff --git a/usr.sbin/pw/tests/pw_modify.sh b/usr.sbin/pw/tests/pw_modify.sh
new file mode 100755
index 0000000..b81f105
--- /dev/null
+++ b/usr.sbin/pw/tests/pw_modify.sh
@@ -0,0 +1,80 @@
+# $FreeBSD$
+
+# Import helper functions
+. $(atf_get_srcdir)/helper_functions.shin
+
+
+# Test adding & removing a user from a group
+atf_test_case groupmod_user
+groupmod_user_body() {
+	populate_etc_skel
+	atf_check -s exit:0 pw -V ${HOME} addgroup test
+	atf_check -s exit:0 pw -V ${HOME} groupmod test -m root
+	atf_check -s exit:0 -o match:"^test:\*:1001:root$" \
+		grep "^test:\*:.*:root$" $HOME/group
+	atf_check -s exit:0 pw -V ${HOME} groupmod test -d root
+	atf_check -s exit:0 -o match:"^test:\*:1001:$" \
+		grep "^test:\*:.*:$" $HOME/group
+}
+
+
+# Test adding and removing a user that does not exist
+atf_test_case groupmod_invalid_user
+groupmod_invalid_user_body() {
+	populate_etc_skel
+	atf_check -s exit:0 pw -V ${HOME} addgroup test
+	atf_check -s exit:67 -e match:"does not exist" pw -V ${HOME} groupmod test -m foo
+	atf_check -s exit:0  pw -V ${HOME} groupmod test -d foo
+}
+
+atf_test_case groupmod_bug_193704
+groupmod_bug_193704_head() {
+	atf_set "descr" "Regression test for the #193704 bug"
+}
+groupmod_bug_193704_body() {
+	populate_etc_skel
+	atf_check -s exit:0 -x pw -V ${HOME} groupadd test
+	atf_check -s exit:0 -x pw -V ${HOME} groupmod test -l newgroupname
+	atf_check -s exit:65 -e match:"^pw: unknown group" -x pw -V ${HOME} groupshow test
+}
+
+atf_test_case usermod_bug_185666
+usermod_bug_185666_head() {
+	atf_set "descr" "Regression test for the #185666 bug"
+}
+
+usermod_bug_185666_body() {
+	populate_etc_skel
+	atf_check -s exit:0 -x pw -V ${HOME} useradd testuser
+	atf_check -s exit:0 -x pw -V ${HOME} groupadd testgroup
+	atf_check -s exit:0 -x pw -V ${HOME} groupadd testgroup2
+	atf_check -s exit:0 -x pw -V ${HOME} usermod testuser -G testgroup
+	atf_check -o inline:"testuser:*:1001:\n" -x pw -V${HOME} groupshow testuser
+	atf_check -o inline:"testgroup:*:1002:testuser\n" -x pw -V ${HOME} groupshow testgroup
+	atf_check -o inline:"testgroup2:*:1003:\n" -x pw -V${HOME} groupshow testgroup2
+	atf_check -s exit:0 -x pw -V ${HOME} usermod testuser -G testgroup2
+	atf_check -o inline:"testuser:*:1001:\n" -x pw -V ${HOME} groupshow testuser
+	atf_check -o inline:"testgroup:*:1002:\n" -x pw -V ${HOME} groupshow testgroup
+	atf_check -o inline:"testgroup2:*:1003:testuser\n" -x pw -V ${HOME} groupshow testgroup2
+}
+
+atf_test_case do_not_duplicate_group_on_gid_change
+do_not_duplicate_group_on_gid_change_head() {
+	atf_set "descr" "Do not duplicate group on gid change"
+}
+
+do_not_duplicate_group_on_gid_change_body() {
+	populate_etc_skel
+	atf_check -s exit:0 -x pw -V ${HOME} groupadd testgroup
+	atf_check -s exit:0 -x pw -V ${HOME} groupmod testgroup -g 12345
+	# use grep to see if the entry has not be duplicated
+	atf_check -o inline:"testgroup:*:12345:\n" -s exit:0 -x grep "^testgroup" ${HOME}/group
+}
+
+atf_init_test_cases() {
+	atf_add_test_case groupmod_user
+	atf_add_test_case groupmod_invalid_user
+	atf_add_test_case groupmod_bug_193704
+	atf_add_test_case usermod_bug_185666
+	atf_add_test_case do_not_duplicate_group_on_gid_change
+}