Bug 169471 - [patch] pw(8) deletes group "username" on userdel even if group "username" is not assoc. w/user "username"
Summary: [patch] pw(8) deletes group "username" on userdel even if group "username" is...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 9.0-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Devin Teske
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-27 00:20 UTC by Devin Teske
Modified: 2018-05-29 00:24 UTC (History)
1 user (show)

See Also:
ngie: mfc-stable8-
bugmeister: mfc-stable9?
ngie: mfc-stable10+


Attachments
patch_bin_169471.txt (1008 bytes, text/plain; charset=US-ASCII)
2012-09-08 20:12 UTC, apyhalov
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Teske freebsd_committer 2012-06-27 00:20:06 UTC
When performing "pw userdel USERNAME", pw(8) will delete a group by the same name regardless of association (or lack thereof) between a group and a user by the same name.

NOTE: Imagine if you had created a user named "wheel" and then executed "pw userdel wheel". The "wheel" group was just deleted silently without warning. No [simple] mechanism is provided to prevent the deletion of the group when deleting a user by the same name.

Fix: 

I see a couple solutions, such as:
1. patch pw(8) to not touch groups during userdel (this requires scripts to adjust if they were relying on this feature), or...
2. patch pw(8) to check that the primary gid of the user being deleted is that of the group sharing the same name as the user.
How-To-Repeat: pw useradd foo -g wheel
# This creates user "foo" with primary gid of existing wheel group (0)
pw groupadd foo
# This adds a group with the same name
pw userdel foo
# This deletes both the user and the group (despite the fact that they are unrelated to each other -- read: user foo was not a member of group foo, nor did it have group foo as its primary gid).
Comment 1 apyhalov 2012-09-08 20:12:52 UTC
This patch should solve the problem.

--
Best regards,
Alexander Pyhalov
Comment 2 dfilter service freebsd_committer 2014-03-13 18:16:50 UTC
Author: dteske
Date: Thu Mar 13 18:16:42 2014
New Revision: 263114
URL: http://svnweb.freebsd.org/changeset/base/263114

Log:
  Fix pw(8) deletion of group "username" on userdel even if group "username"
  is not associated with user "username". E.g., user "foo" has primary group
  "wheel" and is unassociated with group "foo", yet userdel would delete the
  group "foo" when deleting user "foo" (despite the fact that user "foo" is
  not associated with group "foo" in any way).
  
  Patch committed with minor style(9) changes.
  
  PR:		bin/169471
  Submitted by:	Alexander Pyhalov <apyhalov@gmail.com>

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 13 18:11:42 2014	(r263113)
+++ head/usr.sbin/pw/pw_user.c	Thu Mar 13 18:16:42 2014	(r263114)
@@ -380,6 +380,8 @@ pw_user(struct userconf * cnf, int mode,
 			char            file[MAXPATHLEN];
 			char            home[MAXPATHLEN];
 			uid_t           uid = pwd->pw_uid;
+			struct group    *gr;
+			char            grname[LOGNAMESIZE];
 
 			if (strcmp(pwd->pw_name, "root") == 0)
 				errx(EX_DATAERR, "cannot remove user 'root'");
@@ -406,6 +408,11 @@ pw_user(struct userconf * cnf, int mode,
 			 */
 			sprintf(file, "%s/%s", _PATH_MAILDIR, pwd->pw_name);
 			strlcpy(home, pwd->pw_dir, sizeof(home));
+			gr = GETGRGID(pwd->pw_gid);
+			if (gr != NULL)
+				strlcpy(grname, gr->gr_name, LOGNAMESIZE);
+			else
+				grname[0] = '\0';
 
 			rc = delpwent(pwd);
 			if (rc == -1)
@@ -426,7 +433,8 @@ pw_user(struct userconf * cnf, int mode,
 
 			grp = GETGRNAM(a_name->val);
 			if (grp != NULL &&
-			    (grp->gr_mem == NULL || *grp->gr_mem == NULL))
+			    (grp->gr_mem == NULL || *grp->gr_mem == NULL) &&
+			    strcmp(a_name->val, grname) == 0)
 				delgrent(GETGRNAM(a_name->val));
 			SETGRENT();
 			while ((grp = GETGRENT()) != NULL) {
_______________________________________________
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"
Comment 3 Devin Teske freebsd_committer 2014-03-13 18:17:56 UTC
State Changed
From-To: open->patched

Tested submitted patch successfully and committed with minor style(9) 
changes as SVN r263114. Thanks! (and sorry for the ~18mo delay, ugh).
Comment 4 Enji Cooper freebsd_committer 2015-10-25 21:52:53 UTC
- stable/8 is EOL.
- Not MFCed to stable/9.
- MFCed to stable/10 in r272192.

Reassigning to dteske for analysis as r272192 MFCed a number of commits to stable/10.
Comment 5 Devin Teske freebsd_committer 2015-11-02 21:36:54 UTC
Will have a look, thanks Ngie
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:47:32 UTC
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.
Comment 7 Devin Teske freebsd_committer 2018-05-29 00:24:22 UTC
Looks good for 10. Closing as older releases now EOL