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

List:       busybox
Subject:    Re: [PATCH] add/remove-shell: prevent world writable /etc/shells
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2017-05-26 14:46:26
Message-ID: CAK1hOcPQgXWxYhm1ekOsqTnoN5wxBBmH0psGY=_SbJqQPVAv1g () mail ! gmail ! com
[Download RAW message or body]

On Wed, May 10, 2017 at 6:40 PM, Natanael Copa <ncopa@alpinelinux.org> wrote:
> add-shell will not preserve the current permissions, and if umask is 0
> it will create the /etc/shells world writable. To reproduce:
>
>   umask 0; add-shell /bin/bash; ls -l /etc/shells
>
> As a workaround we add the current st_mode with xopen3, which at least
> will prevent /etc/shells to get more permissions than it previously
> had.
>
> Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
> ---
>  loginutils/add-remove-shell.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/loginutils/add-remove-shell.c b/loginutils/add-remove-shell.c
> index af7c31779..a434d054d 100644
> --- a/loginutils/add-remove-shell.c
> +++ b/loginutils/add-remove-shell.c
> @@ -54,6 +54,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
>         FILE *orig_fp;
>         char *orig_fn;
>         char *new_fn;
> +       struct stat sb;
>
>         argv++;
>
> @@ -63,6 +64,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
>         orig_fp = fopen_for_read(orig_fn);
>
>         new_fn = xasprintf("%s.tmp", orig_fn);
> +       xfstat(fileno(orig_fp), &sb, orig_fn);

This wouldn't work if orig_fp is NULL.

I committed a slightly different fix. Yell if it doesn't work.
_______________________________________________
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]