fs: allow protected cross-uid sticky symlinks

A long-standing class of security issues is the symlink-based time-of-check-time-of-use race, most commonly seen in world-writable directories like /tmp. The common method of exploitation of this flaw is to cross privilege boundaries when opening a file through a given symlink (i.e. a root process opens a symlink belonging to another user). For a likely incomplete list of hundreds of examples across the years, please see: http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=/tmp

The solution is to permit symlinks to only be opened when outside a sticky world-writable directory, or when the uid of the symlink and opener match, or when the directory owner matches the symlink's owner.

Some pointers to the history of earlier discussion that I could find:

 1996 Aug, Zygo Blaxell
 http://marc.info/?l=bugtraq&m=87602167419830&w=2
 1996 Oct, Andrew Tridgell
 http://lkml.indiana.edu/hypermail/linux/kernel/9610.2/0086.html
 1997 Dec, Albert D Cahalan
 http://lkml.org/lkml/1997/12/16/4
 2005 Feb, Lorenzo Hernández García-Hierro
 http://lkml.indiana.edu/hypermail/linux/kernel/0502.0/1896.html

Past objections and rebuttals could be summarized as:

 - Violates POSIX.
  - POSIX didn't consider this situation and it's not useful to follow a broken specification at the cost of security.
 - Might break unknown applications that use this feature.
  - Applications that break because of the change are easy to spot and fix. Applications that are vulnerable to symlink ToCToU by not having the change aren't.
 - Applications should just use mkstemp() or O_CREATE|O_EXCL.
  - True, but applications are not perfect, and new software is written all the time that makes these mistakes; blocking this flaw at the kernel is a single solution to the entire class of vulnerability.

This patch is based on the patch in Openwall and grsecurity, but with the scope changed to be only "opening" a symlink.  I have added a sysctl to enable the protected behavior, documentation, and a ratelimited warning.

v2:
 - dropped redundant S_ISLNK check.
 - moved sysctl extern into security.h.

 - asked to include CC to linux-fsdevel.

v3:
 - move into VFS core.
 - add CONFIG entry for build-time default.
 - rename sysctl, invert logic.
 - use get_task_comm for task name.
 - lock dentry when checking parent.

v4:
 - limit check to leaf symlink opening.

Signed-off-by Kees Cook <[email protected]>

---
 Documentation/sysctl/fs.txt |   15 ++++++++++
 fs/Kconfig                  |   15 ++++++++++
 fs/namei.c                  |   61 +++++++++++++++++++++++++++++++++++++++++++ kernel/sysctl.c             |   10 +++++++

 4 files changed, 101 insertions(+), 0 deletions(-)

Documentation/sysctl/fs.txt +0 -0

index 6268250..9986bce 100644
@@ -32,6 +32,7 @@ Currently, these files are in /proc/sys/fs:

 - nr_open
 - overflowuid
 - overflowgid
+- protected-sticky-symlinks
 - suid_dumpable
 - super-max
 - super-nr

Documentation/sysctl/fs.txt +0 -0

@@ -158,6 +159,20 @@ The default is 65534.
 

 ==============================================================

Documentation/sysctl/fs.txt +14 -0

+protected-sticky-symlinks:
+
+Opening symlinks in sticky world-writable directories (like /tmp) can be
+dangerous due to time-of-check-time-of-use races that frequently result
+in security vulnerabilities.
+
+The default value is "0", leaving the behavior of symlink opening
+unchanged from POSIX.  A value of "1" will enable the protection, causing
+symlinks to be openable only if outside a sticky world-writable directory,
+or if the symlink and the opener's uid match, or if the symlink and its
+directory are owned by the same uid.
+
+==============================================================
+

 suid_dumpable:

 This value can be used to query and set the core dump mode for setuid

fs/Kconfig +0 -0

index 5f85b59..48df7cd 100644
@@ -256,3 +256,18 @@ source "fs/nls/Kconfig"

 source "fs/dlm/Kconfig"

 endmenu

fs/Kconfig +15 -0

+
+config PROTECTED_STICKY_SYMLINKS
+       bool "Protect symlink opening in sticky world-writable directories"
+       help
+         A long-standing class of security issues is the symlink-based
+         time-of-check-time-of-use race, most commonly seen in
+         world-writable directories like /tmp. The common method of
+         exploitation of this flaw is to cross privilege boundaries
+         when opening a given symlink (i.e. a root process opens a
+          malicious symlink belonging to another user).
+
+         Enabling this solves the problem by permitting symlinks to only
+         be opened when outside a sticky world-writable directory, or
+          when the uid of the symlink and opener match, or when the
+          directory and symlink owners match.

fs/namei.c +55 -0

index 868d0cb..ee9d493 100644
@@ -32,6 +32,7 @@
  #include <linux/fcntl.h>
  #include <linux/device_cgroup.h>
  #include <linux/fs_struct.h>
+#include <linux/ratelimit.h>
  #include <asm/uaccess.h>
 
  #include "internal.h"

@@ -530,6 +531,60 @@ static inline void path_to_nameidata(struct path *path, struct nameidata *nd)
        nd->path.dentry = path->dentry;
  }
 
+int protected_sticky_symlinks = CONFIG_PROTECTED_STICKY_SYMLINKS;
+
+/**
+ * may_open_sticky_symlink - Check symlink opening for unsafe situations
+ * @dentry: The inode/dentry of the symlink
+ * @nameidata: The path data of the symlink
+ *
+ * In the case of the protected_sticky_symlinks sysctl being enabled,
+ * CAP_DAC_OVERRIDE needs to be specifically ignored if the symlink is
+ * in a sticky world-writable directory.  This is to protect privileged
+ * processes from failing races against path names that may change out
+ * from under them by way of other users creating malicious symlinks.
+ * It will permit symlinks to only be opened when outside a sticky
+ * world-writable directory, or when the uid of the symlink and opener
+ * match, or when the directory owner matches the symlink's owner.
+ *
+ * Returns 0 if opening the symlink is allowed, -ve on error.
+ */
+static __always_inline int
+may_open_sticky_symlink(struct dentry *dentry, struct nameidata *nameidata)
+{
+       int error = 0;
+       const struct inode *parent;
+       const struct inode *inode;
+       const struct cred *cred;
+
+       if (!protected_sticky_symlinks)
+               return 0;
+
+       /* owner and opener match? */
+       cred = current_cred();
+       inode = dentry->d_inode;
+       if (cred->fsuid == inode->i_uid)
+               return 0;
+
+       /* check parent directory mode and owner */
+       spin_lock(&dentry->d_lock);
+       parent = dentry->d_parent->d_inode;
+       if ((parent->i_mode & (S_ISVTX|S_IWOTH)) == (S_ISVTX|S_IWOTH) &&
+           parent->i_uid != inode->i_uid) {
+               error = -EACCES;
+       }
+       spin_unlock(&dentry->d_lock);
+
+       if (error) {
+               char name[sizeof(current->comm)];
+               printk_ratelimited(KERN_NOTICE "non-matching-uid symlink "
+                       "opening attempted in sticky world-writable "
+                       "directory by %s (fsuid %d)\n",
+                       get_task_comm(name, current), cred->fsuid);
+       }
+       return error;
+}
+

 static __always_inline int
 __do_follow_link(struct path *path, struct nameidata *nd, void **p) {

fs/namei.c +6 -0

@@ -1844,6 +1899,12 @@ reval:
                        goto exit_dput;
                if (count++ == 32)
                        goto exit_dput;
+
+               /* check if this symlink is in a sticky world-write dir */
+               error = may_open_sticky_symlink(path.dentry, &nd);
+               if (error)
+                       goto exit_dput;
+
                /*
                 * This is subtle. Instead of calling do_follow_link() we do
                 * the thing by hands. The reason is that this way we have zero

kernel/sysctl.c +10 -1

index 997080f..56affd6 100644
@@ -87,6 +87,7 @@ extern int sysctl_oom_kill_allocating_task;
  extern int sysctl_oom_dump_tasks;
  extern int max_threads;
  extern int core_uses_pid;
+extern int protected_sticky_symlinks;
  extern int suid_dumpable;
  extern char core_pattern[];
  extern unsigned int core_pipe_limit;

@@ -1455,6 +1456,15 @@ static struct ctl_table fs_table[] = {
  #endif
  #endif
        {
+               .procname       = "protected-sticky-symlinks",
+               .data           = &protected_sticky_symlinks,
+               .maxlen         = sizeof(int),
+               .mode           = 0644,
+               .proc_handler   = proc_dointvec_minmax,
+               .extra1         = &zero,
+               .extra2         = &one,
+       },
+       {
                .procname       = "suid_dumpable",
                .data           = &suid_dumpable,
                .maxlen         = sizeof(int),
--

1.7.0.4

--
Kees Cook
Ubuntu Security Team

Quoted 2 lines · Kees Cook

v2:
 - moved sysctl extern into security.h.

Not in v4?

Quoted 3 lines

v3:
 - move into VFS core.
 - rename sysctl, invert logic.

Hi,

Quoted 4 lines · Dave Young

Quoted 2 lines · Kees Cook

v2:
 - moved sysctl extern into security.h.

Quoted 5 lines

Not in v4?

Quoted 3 lines

v3:
 - move into VFS core.
 - rename sysctl, invert logic.

Right, as part of the "move out of security/" the sysctl moved to live in fs/namei.c (and therefore needed an extern in kernel/sysctl.c).

-Kees

--
Kees Cook
Ubuntu Security Team

Quoted 11 lines · Kees Cook

Quoted 9 lines · Dave Young

Quoted 2 lines · Kees Cook

v2:
 - moved sysctl extern into security.h.

Not in v4?

Quoted 3 lines

v3:
 - move into VFS core.
 - rename sysctl, invert logic.

Quoted 2 lines

Right, as part of the "move out of security/" the sysctl moved to live in fs/namei.c (and therefore needed an extern in kernel/sysctl.c).

Then linux/fs.h is the place to go
sysctl_nr_open_min and sysctl_nr_open_max can go there as well.