This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] BZ #15755: CVE-2013-2207: pt_chown tricked into granting access to another users pseudo-terminal


CVE-2013-2207: pt_chown tricked into granting access to another 
users pseudo-terminal

Pre-conditions for the attack:

 * Attacker with local user account
 * Kernel with FUSE support
 * "user_allow_other" in /etc/fuse.conf
 * Victim with allocated slave in /dev/pts

Using the setuid installed pt_chown and a weak check on whether a file
descriptor is a tty, an attacker could fake a pty check using FUSE and
trick pt_chown to grant ownership of a pty descriptor that the current
user does not own.  It cannot access /dev/pts/ptmx however.

pt_chown is not needed in most modern distributions since devpts is
enabled by default.  So the fix is to add a configure option to
enable building pt_chown.  This means that pt_chown will not be built
by default.  Distributions will be required to avoid installing
pt_chown in that case.

There is further discussion to be had around what is or is not valid
for a FUSE filesystem to do and how glibc can help enforce some of that
security in tcgetattr. However first things first we need to disable
the use of pt_chown by default.

Siddhesh is out so I'm submitting this on his behalf.

OK to commit?

Tested on x86-64 with and without --enable-pt_chown and verified
code is enabled and then removed, and that pt_chown is built and
then not built.

Verified manual looks good and findex shows two entries for grantpt.

NEWS

* CVE-2013-2207 Granting access to another user's pseudo-terminal
  has been fixed by disabling pt_chown (Bugzilla #15755).

2013-07-19  Siddhesh Poyarekar  <siddhesh@redhat.com>
	    Andreas Schwab  <schwab@suse.de>
	    Roland McGrath  <roland@hack.frob.com>
	    Joseph Myers  <joseph@codesourcery.com>
	    Carlos O'Donell  <carlos@redhat.com>

	* config.h.in: Define HAVE_PT_CHOWN.
	* config.make.in (build-pt-chown): New variable.
	* configure.in (--enable-pt_chown): New configure option.
	* configure: Regenerate.
	* login/Makefile: Include Makeconfig.  Build pt_chown only if
	build-pt-chown is enabled.
	* sysdeps/unix/grantpt.c (grantpt) [HAVE_PT_CHOWN]: Spawn
	pt_chown to fix pty ownership.
	* sysdeps/unix/sysv/linux/grantpt.c [HAVE_PT_CHOWN]: Define
	CLOSE_ALL_FDS.
	* manual/install.texi (Configuring and compiling): Mention
	--enable-pt_chown. Add @findex for grantpt.
	* INSTALL: Regenerate.

diff --git a/config.h.in b/config.h.in
index 6284e2a..a85f131 100644
--- a/config.h.in
+++ b/config.h.in
@@ -238,4 +238,7 @@
 /* The ARM hard-float ABI is being used.  */
 #undef HAVE_ARM_PCS_VFP
 
+/* The pt_chown binary is being built and used by grantpt.  */
+#undef HAVE_PT_CHOWN
+
 #endif
diff --git a/config.make.in b/config.make.in
index b01b70b..7b04568 100644
--- a/config.make.in
+++ b/config.make.in
@@ -95,6 +95,7 @@ link-obsolete-rpc = @link_obsolete_rpc@
 build-nscd = @build_nscd@
 use-nscd = @use_nscd@
 build-hardcoded-path-in-tests= @hardcoded_path_in_tests@
+build-pt-chown = @build_pt_chown@
 
 # Build tools.
 CC = @CC@
diff --git a/configure.in b/configure.in
index 4db1acf..769e8ef 100644
--- a/configure.in
+++ b/configure.in
@@ -353,6 +353,16 @@ AC_ARG_ENABLE([nscd],
 	      [use_nscd=$enableval],
 	      [use_nscd=yes])
 
+AC_ARG_ENABLE([pt_chown],
+	      [AS_HELP_STRING([--enable-pt_chown],
+	       [Enable building and installing pt_chown])],
+	      [build_pt_chown=$enableval],
+	      [build_pt_chown=no])
+AC_SUBST(build_pt_chown)
+if test $build_pt_chown = yes; then
+  AC_DEFINE(HAVE_PT_CHOWN)
+fi
+
 # The way shlib-versions is used to generate soversions.mk uses a
 # fairly simplistic model for name recognition that can't distinguish
 # i486-pc-linux-gnu fully from i486-pc-gnu.  So we mutate a $host_os
diff --git a/login/Makefile b/login/Makefile
index 0bfe643..430c6d9 100644
--- a/login/Makefile
+++ b/login/Makefile
@@ -30,9 +30,15 @@ routines := getlogin getlogin_r setlogin getlogin_r_chk \
 
 CFLAGS-grantpt.c = -DLIBEXECDIR='"$(libexecdir)"'
 
-others = utmpdump pt_chown
+others = utmpdump
+
+include ../Makeconfig
+
+ifeq (yes,$(build-pt-chown))
+others += pt_chown
 others-pie = pt_chown
 install-others-programs = $(inst_libexecdir)/pt_chown
+endif
 
 subdir-dirs = programs
 vpath %.c programs
diff --git a/manual/install.texi b/manual/install.texi
index 0c05f51..4575d22 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -163,6 +163,20 @@ so that they can be invoked directly.
 @item --enable-lock-elision=yes
 Enable lock elision for pthread mutexes by default.
 
+@pindex pt_chown
+@findex grantpt
+@item --enable-pt_chown
+The file @file{pt_chown} is a helper binary for @code{grantpt}
+(@pxref{Allocation, Pseudo-Terminals}) that is installed setuid root to
+fix up pseudo-terminal ownership.  It is not built by default because
+systems using the Linux kernel are commonly built with the @code{devpts}
+filesystem enabled and mounted at @file{/dev/pts}, which manages
+pseudo-terminal ownership automatically.  By using
+@samp{--enable-pt_chown}, you may build @file{pt_chown} and install it
+setuid and owned by @code{root}.  The use of @file{pt_chown} introduces
+additional security risks to the system and you should enable it only if
+you understand and accept those risks.
+
 @item --build=@var{build-system}
 @itemx --host=@var{host-system}
 These options are for cross-compiling.  If you specify both options and
diff --git a/sysdeps/unix/grantpt.c b/sysdeps/unix/grantpt.c
index d37da13..431be85 100644
--- a/sysdeps/unix/grantpt.c
+++ b/sysdeps/unix/grantpt.c
@@ -173,9 +173,10 @@ grantpt (int fd)
   retval = 0;
   goto cleanup;
 
-  /* We have to use the helper program.  */
+  /* We have to use the helper program if it is available.  */
  helper:;
 
+#ifdef HAVE_PT_CHOWN
   pid_t pid = __fork ();
   if (pid == -1)
     goto cleanup;
@@ -190,9 +191,9 @@ grantpt (int fd)
 	if (__dup2 (fd, PTY_FILENO) < 0)
 	  _exit (FAIL_EBADF);
 
-#ifdef CLOSE_ALL_FDS
+# ifdef CLOSE_ALL_FDS
       CLOSE_ALL_FDS ();
-#endif
+# endif
 
       execle (_PATH_PT_CHOWN, basename (_PATH_PT_CHOWN), NULL, NULL);
       _exit (FAIL_EXEC);
@@ -231,6 +232,7 @@ grantpt (int fd)
 	    assert(! "getpt: internal error: invalid exit code from pt_chown");
 	  }
     }
+#endif
 
  cleanup:
   if (buf != _buf)
diff --git a/sysdeps/unix/sysv/linux/grantpt.c b/sysdeps/unix/sysv/linux/grantpt.c
index 0a3cd47..8cebde3 100644
--- a/sysdeps/unix/sysv/linux/grantpt.c
+++ b/sysdeps/unix/sysv/linux/grantpt.c
@@ -11,7 +11,7 @@
 
 #include "pty-private.h"
 
-
+#if HAVE_PT_CHOWN
 /* Close all file descriptors except the one specified.  */
 static void
 close_all_fds (void)
@@ -38,6 +38,7 @@ close_all_fds (void)
       __dup2 (STDOUT_FILENO, STDERR_FILENO);
     }
 }
-#define CLOSE_ALL_FDS() close_all_fds()
+# define CLOSE_ALL_FDS() close_all_fds()
+#endif
 
 #include <sysdeps/unix/grantpt.c>
---

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]