[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH v2] deluser: check if specified home is a directory before removing it
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2020-06-09 16:05:41
Message-ID: CAK1hOcNxe07knar8k5Pq+cOudAmdvczdqeiF=tcFjjEuPnny_w () mail ! gmail ! com
[Download RAW message or body]

Applied.

You want if (stat(pw->pw_dir, &st) == 0 && S_ISDIR(st.st_mode))
in this case: the directory may be deleted already, should not bomb
out in this case.

On Tue, Jun 9, 2020 at 5:52 PM <soeren@soeren-tempel.net> wrote:
> 
> From: Sören Tempel <soeren+git@soeren-tempel.net>
> 
> On Alpine, some users use /dev/null as a home directory. When removing
> such a user with `deluser --remove-home` this causes the /dev/null
> device file to be removed which is undesirable. To prevent this pitfall,
> check if the home directory specified for the user is an actual
> directory (or a symlink to a directory).
> 
> Implementations of similar tools for other operating systems also
> implement such checks. For instance, the OpenBSD rmuser(1)
> implementation [0].
> 
> [0]: https://github.com/openbsd/src/blob/b69faa6c70c5bfcfdddc6138cd8e0ee18cc15b03/usr.sbin/adduser/rmuser.perl#L143-L151
>                 
> ---
> loginutils/deluser.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/loginutils/deluser.c b/loginutils/deluser.c
> index 56bc7eaa6..585e82090 100644
> --- a/loginutils/deluser.c
> +++ b/loginutils/deluser.c
> @@ -99,8 +99,15 @@ int deluser_main(int argc, char **argv)
> pfile = bb_path_passwd_file;
> if (ENABLE_FEATURE_SHADOWPASSWDS)
> sfile = bb_path_shadow_file;
> -                       if (opt_delhome)
> -                               remove_file(pw->pw_dir, FILEUTILS_RECUR);
> +                       if (opt_delhome) {
> +                               struct stat st;
> +
> +                               /* Make sure home is an actual directory before
> +                                * removing it (e.g. users with /dev/null as home) \
> */ +                               xstat(pw->pw_dir, &st);
> +                               if (S_ISDIR(st.st_mode))
> +                                       remove_file(pw->pw_dir, FILEUTILS_RECUR);
> +                       }
> } else {
> struct group *gr;
> do_delgroup:
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]