From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: [PATCH v2] fs: block cross-uid sticky symlinks Date: Sun, 30 May 2010 20:50:53 -0700 Message-ID: References: <20100531030402.GQ6056@outflux.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-doc@vger.kernel.org, Randy Dunlap , Andrew Morton , Jiri Kosina , Dave Young , Martin Schwidefsky , James Morris , Eric Paris , David Howells , Ingo Molnar , Peter Zijlstra , Tim Gardner , "Serge E. Hallyn" To: Kees Cook Return-path: In-Reply-To: <20100531030402.GQ6056@outflux.net> (Kees Cook's message of "Sun\, 30 May 2010 20\:04\:02 -0700") Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Kees Cook 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 lik= ely > incomplete list of hundreds of examples across the years, please see: > http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=3D/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=3Dbugtraq&m=3D87602167419830&w=3D2 > 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=C3=A1ndez Garc=C3=ADa-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 foll= ow > 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 a= nd > fix. Applications that are vulnerable to symlink ToCToU by not h= aving > the change aren't. > - Applications should just use mkstemp() or O_CREATE|O_EXCL. > - True, but applications are not perfect, and new software is writ= ten > all the time that makes these mistakes; blocking this flaw at th= e > kernel is a single solution to the entire class of vulnerability= =2E > > 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 se= parate /tmp directories per user? That works today, with no kernel changes. Placing this in cap_inode_follow_link is horrible naming. There is not= hing 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.=20 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