Re: Bug#1013924: coreutils: runcon -c getfscon()s program verbatim but execve()s it; trojan moment?

[Pádraig Brady]

On 27/06/2022 19:13, Pádraig Brady wrote:
> On 27/06/2022 16:04, наб wrote:
> > Package: coreutils
> > Version: 8.32-4+b1
> > Severity: normal
> >
> > Dear Maintainer,
> >
> > The strace for runcon -c true true (after a > true) contains
> >     getxattr("true", "security.selinux", "unconfined_u:object_r:user_tmp_t", 
> > 255) = 36
> >     execve("/usr/local/sbin/true", ["true", "true"]) = -1 ENOENT
> >     execve("/usr/local/bin/true", ["true", "true"]) = -1 ENOENT
> >     execve("/sbin/true", ["true", "true"]) = -1 ENOENT
> >     execve("/bin/true", ["true", "true"]) = 0
> >
> > This corresponds to getfscon("true"), execvp("true", ["true", NULL]).
> > (of course, this also errors if ./true doesn't exist).
> >
> > So, uh: is this intentional? It certainly feels wrong? All invocations
> > take a PATH executable except this one which takes a PATH executable
> > that must *also* be a valid file? And also invites a trivial trojan
> > because the precomputed transition is to the file in the cwd, but the
> > program executed lives somewhere in PATH? Should -c just execv()
> > instead? Am I misunderstanding the usefulness of this?
> >
> > Best,
> > наб
>
> This is a fair point.
> I.e. the following patch would be more correct operation.
> I'll propose this upstream.
>
> Now existing scripts would need to pass absolute paths to `runcon -c`
> to work in the first place, so I don't know how much of a security
> issue this is in practice.
>
> thanks,
> Pádraig
>
> iff --git a/src/runcon.c b/src/runcon.c
> index c4227c784..d85411c79 100644
> --- a/src/runcon.c
> +++ b/src/runcon.c
> @@ -255,7 +255,7 @@ main (int argc, char **argv)
>      if (cur_context != NULL)
>        freecon (cur_context);
>
> -  execvp (argv[optind], argv + optind);
> +  (compute_trans ? execv : execvp) (argv[optind], argv + optind);
>
>      int exit_status = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE;
>      error (0, errno, "%s", quote (argv[optind]));

Pushed the attached upstream.

thanks,
Pádraig

runcon-c-nopath.patch
Description: Text Data