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(-)
diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
index 6268250..9986bce 100644
--- a/Documentation/sysctl/fs.txt
+++ b/Documentation/sysctl/fs.txt
@@ -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
@@ -158,6 +159,20 @@ The default is 65534.
==============================================================
+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
diff --git a/fs/Kconfig b/fs/Kconfig
index 5f85b59..48df7cd 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -256,3 +256,18 @@ source "fs/nls/Kconfig"
source "fs/dlm/Kconfig"
endmenu
+
+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.
diff --git a/fs/namei.c b/fs/namei.c
index 868d0cb..ee9d493 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -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)
{
@@ -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
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 997080f..56affd6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -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
On Thu, Jun 3, 2010 at 6:23 AM, Kees Cook <[email protected]> wrote:
> 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.
Not in v4?
> - 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(-)
>
> diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
> index 6268250..9986bce 100644
> --- a/Documentation/sysctl/fs.txt
> +++ b/Documentation/sysctl/fs.txt
> @@ -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
> @@ -158,6 +159,20 @@ The default is 65534.
>
> ==============================================================
>
> +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
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 5f85b59..48df7cd 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -256,3 +256,18 @@ source "fs/nls/Kconfig"
> source "fs/dlm/Kconfig"
>
> endmenu
> +
> +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.
> diff --git a/fs/namei.c b/fs/namei.c
> index 868d0cb..ee9d493 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -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)
> {
> @@ -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
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 997080f..56affd6 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -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
>
--
Regards
dave
Hi,
On Thu, Jun 03, 2010 at 08:51:28AM +0800, Dave Young wrote:
> On Thu, Jun 3, 2010 at 6:23 AM, Kees Cook <[email protected]> wrote:
> > v2:
> > ?- moved sysctl extern into security.h.
>
> Not in v4?
>
> > 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
On Thu, Jun 3, 2010 at 9:18 AM, Kees Cook <[email protected]> wrote:
> Hi,
>
> On Thu, Jun 03, 2010 at 08:51:28AM +0800, Dave Young wrote:
>> On Thu, Jun 3, 2010 at 6:23 AM, Kees Cook <[email protected]> wrote:
>> > v2:
>> > - moved sysctl extern into security.h.
>>
>> Not in v4?
>>
>> > 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).
Then linux/fs.h is the place to go
sysctl_nr_open_min and sysctl_nr_open_max can go there as well.
>
> -Kees
>
> --
> Kees Cook
> Ubuntu Security Team
>
--
Regards
dave
On Thu, Jun 03, 2010 at 09:42:21AM +0800, Dave Young wrote:
> On Thu, Jun 3, 2010 at 9:18 AM, Kees Cook <[email protected]> wrote:
> > On Thu, Jun 03, 2010 at 08:51:28AM +0800, Dave Young wrote:
> >> On Thu, Jun 3, 2010 at 6:23 AM, Kees Cook <[email protected]> wrote:
> >> > v2:
> >> > ?- moved sysctl extern into security.h.
> >>
> >> Not in v4?
> >>
> >> > 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).
>
> Then linux/fs.h is the place to go
> sysctl_nr_open_min and sysctl_nr_open_max can go there as well.
Okay, I wasn't sure if that was right or not. I'll do that for v6, and
send the sysctl_nr_open_* in a separate patch.
Thanks!
-Kees
--
Kees Cook
Ubuntu Security Team