Re: [PATCH] chown: prevent multiple --from options

[Pádraig Brady]

On 29/09/19 10:46, Francois wrote:
> chown --from option can be used to restrict the list of files
> concerned by the chown.
> 
> However this is true only for the latest --from option provided on the
> command line. One can run chown --from=1000:1000 --from=0:0, in that
> case the first --from will be silently discarded.
> 
> It is an issue on systems where sysadmins naively provide a sudo rule
> for their users expecting to restrict its usage for a specific owner.
> 
> We can fix by rejecting the cases where --from option is provided multiple
> times and uid or gid are set twice.
> 
> * src/chown.c: prevent multiple --from options
> 
> * tests/chown/basic.sh: prevent multiple --from options
> ---
>  src/chown.c          | 14 ++++++++++++--
>  tests/chown/basic.sh |  9 +++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/chown.c b/src/chown.c
> index c1db307ab..a06bf6ed4 100644
> --- a/src/chown.c
> +++ b/src/chown.c
> @@ -227,11 +227,21 @@ main (int argc, char **argv)
> 
>          case FROM_OPTION:
>            {
> -            const char *e = parse_user_spec (optarg,
> -                                             &required_uid, &required_gid,
> +            const char *e;
> +            uid_t parsed_required_uid = -1;
> +            gid_t parsed_required_gid = -1;
> +            e = parse_user_spec (optarg,
> +                                             &parsed_required_uid,
> +                                             &parsed_required_gid,
>                                               NULL, NULL);
>              if (e)
>                die (EXIT_FAILURE, 0, "%s: %s", e, quote (optarg));
> +            if ((parsed_required_uid != -1 && required_uid != -1) ||
> +                (parsed_required_gid != -1 && required_gid != -1))
> +              die (EXIT_FAILURE, 0, "%s",
> +                   _("from option provided multiple times"));
> +            required_uid = parsed_required_uid;
> +            required_gid = parsed_required_gid;
>              break;
>            }
> 
> diff --git a/tests/chown/basic.sh b/tests/chown/basic.sh
> index faeeb82aa..2ccfd959d 100755
> --- a/tests/chown/basic.sh
> +++ b/tests/chown/basic.sh
> @@ -56,4 +56,13 @@ chown --no-dereference --from=0:1 2:010 slink || fail=1
>  # owner/group on the symlink should be changed
>  set _ $(ls -n slink); shift; test "$3:$4" = 2:10 || fail=1
> 
> +# Ensure two from are reported properly as error
> +returns_ 1 chown --from=0:0 --from=1:1 0 f > out 2>&1|| fail=1
> +printf "chown: from option provided multiple times\n" > exp
> +compare exp out || fail=1
> +
> +# Unless they set different attributes
> +chown --from=0 --from=:0 0 f  || fail=1
> +
> +
>  Exit $fail
> --
> 2.16.4
> 
> 

I'm inclined to agree with this patch,
as I can't see much use for overriding --from

thanks!
Pádraig