Bug 244967
| Summary: | pw: userdel will delete a home directory not owned by the user (if it is empty) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Eric Hanneken <eric> | ||||||
| Component: | bin | Assignee: | Baptiste Daroussin <bapt> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Some People | CC: | emaste, koobs | ||||||
| Priority: | Normal | Flags: | koobs:
mfc-stable12+
koobs: mfc-stable11+ |
||||||
| Version: | 12.1-RELEASE | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
Thank you for the report and patch Eric. Could you update your patch to be against svn base root please? Thanks :) Comment on attachment 212604 [details]
Patch to pw(8)
Index: usr.sbin/pw/rm_r.c
===================================================================
--- usr.sbin/pw/rm_r.c (revision 359195)
+++ usr.sbin/pw/rm_r.c (working copy)
@@ -71,5 +71,8 @@
closedir(d);
if (fstatat(rootfd, path, &st, AT_SYMLINK_NOFOLLOW) != 0)
return;
- unlinkat(rootfd, path, S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0);
+ if (S_ISLNK(st.st_mode))
+ unlinkat(rootfd, path, 0);
+ else if (st.st_uid == uid)
+ unlinkat(rootfd, path, AT_REMOVEDIR);
}
(In reply to Kubilay Kocak from comment #1) Done. Comment on attachment 212604 [details] Patch to pw(8) >Index: rm_r.c >=================================================================== >--- rm_r.c (revision 359195) >+++ rm_r.c (working copy) >@@ -71,5 +71,8 @@ > closedir(d); > if (fstatat(rootfd, path, &st, AT_SYMLINK_NOFOLLOW) != 0) > return; >- unlinkat(rootfd, path, S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0); >+ if (S_ISLNK(st.st_mode)) >+ unlinkat(rootfd, path, 0); >+ else if (st.st_uid == uid) >+ unlinkat(rootfd, path, AT_REMOVEDIR); > } Created attachment 212605 [details]
Patch to pw(8)
Thank you Eric A commit references this bug: Author: bapt Date: Mon Mar 23 08:23:23 UTC 2020 New revision: 359232 URL: https://svnweb.freebsd.org/changeset/base/359232 Log: pw: do not removed home directories if not owned When deleting a user, if its home directory does not belong to it, it should not be removed. This is the promise that the manpage makes, the tool should ensure that it respects that promise. Add a regression test about it PR: 244967 Submitted by: Eric Hanneken <eric@erichanneken.com> MFC after: 3 days Changes: head/usr.sbin/pw/rm_r.c head/usr.sbin/pw/tests/pw_userdel_test.sh A commit references this bug: Author: bapt Date: Fri Apr 3 05:06:09 UTC 2020 New revision: 359588 URL: https://svnweb.freebsd.org/changeset/base/359588 Log: MFC r359232: pw: do not removed home directories if not owned When deleting a user, if its home directory does not belong to it, it should not be removed. This is the promise that the manpage makes, the tool should ensure that it respects that promise. Add a regression test about it PR: 244967 Submitted by: Eric Hanneken <eric@erichanneken.com> Changes: _U stable/11/ stable/11/usr.sbin/pw/rm_r.c stable/11/usr.sbin/pw/tests/pw_userdel_test.sh A commit references this bug: Author: bapt Date: Fri Apr 3 05:08:07 UTC 2020 New revision: 359589 URL: https://svnweb.freebsd.org/changeset/base/359589 Log: MFC r359232: pw: do not removed home directories if not owned When deleting a user, if its home directory does not belong to it, it should not be removed. This is the promise that the manpage makes, the tool should ensure that it respects that promise. Add a regression test about it PR: 244967 Submitted by: Eric Hanneken <eric@erichanneken.com> Changes: _U stable/12/ stable/12/usr.sbin/pw/rm_r.c stable/12/usr.sbin/pw/tests/pw_userdel_test.sh |
Created attachment 212604 [details] Patch to pw(8) According to the man page for pw(8), pw userdel -n name -r will only remove files and directories that are actually owned by the user, or symbolic links owned by anyone under the user's home directory. The man page for rmuser(8), which wraps pw(8), makes a similar promise. In reality, the ownership check is performed only for files in the user's home directory. The home directory itself, and any subdirectories, are deleted without regard to who owns them. In most cases this is harmless because a directory not owned by the user will usually also contain files not owned by the user. pw won't delete the files, and its attempt to remove a non-empty directory will fail. But if the other-owned home directory is empty (or if its files are weirdly all owned by the user being deleted), pw will delete it: # mkdir /var/shared # ls -ld /var/shared drwxr-xr-x 2 root wheel 2 Mar 21 23:40 /var/shared # pw useradd -n testuser1 -d /var/shared # pw useradd -n testuser2 -d /var/shared # grep 'testuser' /etc/passwd testuser1:*:1002:1002:User &:/var/shared:/bin/sh testuser2:*:1003:1003:User &:/var/shared:/bin/sh # pw userdel -n testuser1 -r # grep 'testuser' /etc/passwd testuser2:*:1003:1003:User &:/var/shared:/bin/sh # ls -ld /var/shared ls: /var/shared: No such file or directory I have attached a patch which adds the ownership check to directories. It was made against revision 359195.