Re: [PATCH v2] fs: block cross-uid sticky symlinks

[Eric W. Biederman]

Kees Cook <kees.cook@xxxxxxxxxxxxx> writes:

> 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 following a given symlink (i.e. a
> root process follows 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 followed when outside a sticky
> world-writable directory, or when the uid of the symlink and follower 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.  I have
> added a sysctl to toggle the behavior back to the old logic via
> /proc/sys/fs/weak-sticky-symlinks, documentation, and a ratelimited
> warning.

The name of the sysctl is horrible it is a double negative, which
makes thinking about it hard.

Why not simply put each user in a different mount namespace and have separate
/tmp directories per user?  That works today, with no kernel changes.

Placing this in cap_inode_follow_link is horrible naming.  There is nothing
capabilities about this.  Either this needs to go into one or several
of the security modules or this needs to go into the core vfs. 

I can't argue with taking action to close the too frequently security
issues in /tmp, but this changes appears to be unnecessary, difficult
to maintain, and difficult to understand.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/