Subject: bug#24541: runcon tty hijacking via TIOCSTI ioctl

From: Pádraig Brady <[email-masked]>
Date: Mon, 28 Aug 2017 02:51:12 -0700

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?= <[email-masked]>
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 <getopt.h>
 #include <selinux/selinux.h>
 #include <selinux/context.h>
+#ifdef HAVE_SECCOMP
+# include <seccomp.h>
+# include <sys/ioctl.h>
+#endif
 #include <sys/types.h>
 #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
+  /* This may have unwanted side effects, but is a fallback
+     on older systems without libseccomp.  */
+  if (setsid () != 0)
+    die (EXIT_FAILURE, errno, _("cannot create session"));
+#endif /* HAVE_SECCOMP */
+}
+
+
 int
 main (int argc, char **argv)
 {
@@ -195,6 +221,8 @@ main (int argc, char **argv)
     die (EXIT_FAILURE, 0, _("%s may be used only on a SELinux kernel"),
          program_name);
 
+  disable_tty_inject ();
+
   if (context)
     {
       con = context_new (context);
diff --git a/tests/local.mk b/tests/local.mk
index fd4713d..f904ffb 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -333,6 +333,7 @@ all_tests =					\
   tests/misc/readlink-root.sh			\
   tests/misc/realpath.sh			\
   tests/misc/runcon-no-reorder.sh		\
+  tests/misc/runcon-no-inject.sh		\
   tests/misc/sha1sum.pl				\
   tests/misc/sha1sum-vec.pl			\
   tests/misc/sha224sum.pl			\
diff --git a/tests/misc/runcon-no-inject.sh b/tests/misc/runcon-no-inject.sh
new file mode 100755
index 0000000..f1ea6ec
--- /dev/null
+++ b/tests/misc/runcon-no-inject.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+# Ensure that runcon does not reorder its arguments.
+
+# Copyright (C) 2017 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ runcon
+
+cat <<\EOF >inject.py || framework_failure_
+import fcntl, termios
+fcntl.ioctl(0, termios.TIOCSTI, '\n')
+EOF
+
+python inject.py || skip_ 'python TIOCSTI check failed'
+
+returns_ 1 runcon $(id -Z) python inject.py || fail=1
+
+Exit $fail
-- 
2.9.3