[PATCH weston] weston-launch: Let the user use her real shell

[David Herrmann]

Hi

On Fri, Jan 24, 2014 at 7:13 AM, Quentin Glidic
<sardemff7+wayland at sardemff7.net> wrote:
> From: Quentin Glidic <sardemff7+git at sardemff7.net>
>
> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> ---
>  src/weston-launch.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/weston-launch.c b/src/weston-launch.c
> index 56e22b1..fd04552 100644
> --- a/src/weston-launch.c
> +++ b/src/weston-launch.c
> @@ -103,6 +103,7 @@ struct weston_launch {
>
>         pid_t child;
>         int verbose;
> +       int user_shell;
>         char *new_user;
>  };
>
> @@ -644,7 +645,7 @@ launch_compositor(struct weston_launch *wl, int argc, char *argv[])
>         sigaddset(&mask, SIGINT);
>         sigprocmask(SIG_UNBLOCK, &mask, NULL);
>
> -       child_argv[0] = "/bin/sh";
> +       child_argv[0] = wl->user_shell ? wl->pw->pw_shell : "/bin/sh";

weston-launch passes privileged FDs back to weston. I don't think we
should allow a user to use their own shell to spawn weston. How can
you guarantee the shell does not point to some random binary of the
user? This way, they can get access to arbitrary input FDs,
circumventing the access-mode restrictions on the device-nodes.

Thanks
David

>         child_argv[1] = "-l";
>         child_argv[2] = "-c";
>         child_argv[3] = BINDIR "/weston \"$@\"";
> @@ -663,6 +664,7 @@ help(const char *name)
>         fprintf(stderr, "Usage: %s [args...] [-- [weston args..]]\n", name);
>         fprintf(stderr, "  -u, --user      Start session as specified username\n");
>         fprintf(stderr, "  -t, --tty       Start session on alternative tty\n");
> +       fprintf(stderr, "  -s, --shell     Use the user shell instead of /bin/sh to run weston\n");
>         fprintf(stderr, "  -v, --verbose   Be verbose\n");
>         fprintf(stderr, "  -h, --help      Display this help message\n");
>  }
> @@ -676,6 +678,7 @@ main(int argc, char *argv[])
>         struct option opts[] = {
>                 { "user",    required_argument, NULL, 'u' },
>                 { "tty",     required_argument, NULL, 't' },
> +               { "shell",   no_argument,       NULL, 's' },
>                 { "verbose", no_argument,       NULL, 'v' },
>                 { "help",    no_argument,       NULL, 'h' },
>                 { 0,         0,                 NULL,  0  }
> @@ -683,7 +686,7 @@ main(int argc, char *argv[])
>
>         memset(&wl, 0, sizeof wl);
>
> -       while ((c = getopt_long(argc, argv, "u:t::vh", opts, &i)) != -1) {
> +       while ((c = getopt_long(argc, argv, "u:t::svh", opts, &i)) != -1) {
>                 switch (c) {
>                 case 'u':
>                         wl.new_user = optarg;
> @@ -693,6 +696,9 @@ main(int argc, char *argv[])
>                 case 't':
>                         tty = optarg;
>                         break;
> +               case 's':
> +                       wl.user_shell = 1;
> +                       break;
>                 case 'v':
>                         wl.verbose = 1;
>                         break;
> --
> 1.8.5.3
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel