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

[Pádraig Brady]

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]));