LWN.net Logo

Filesystem linking protections

From:  Lorenzo =?ISO-8859-1?Q?Hern=E1ndez_?= =?ISO-8859-1?Q?Garc=EDa-Hierro?= <lorenzo@gnu.org>
To:  "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject:  [PATCH] Filesystem linking protections
Date:  Mon, 07 Feb 2005 19:57:06 +0100
Cc:  Chris Wright <chrisw@osdl.org>

Hi,

This patch adds two checks to do_follow_link() and sys_link(), for
prevent users to follow (untrusted) symlinks owned by other users in
world-writable +t directories (i.e. /tmp), unless the owner of the
symlink is the owner of the directory, users will also not be able to
hardlink to files they do not own.

The direct advantage of this pretty simple patch is that /tmp races will
be prevented.

Results reported by the Collision Regression Test Suite with patch
applied:
 (...)
 Symlink restrictions                     : Not vulnerable
 Hardlinking restrictions                 : Not vulnerable
 (...)
Results with patch *not applied*:
 (...)
 Symlink restrictions                     : Vulnerable
 Hardlinking restrictions                 : Vulnerable
 (...)

This patch is based on grSecurity linking protections, but first
implemented by the OpenWall patch.

I propose it's merging, as the overhead is *minimal* (if there's any
overhead), because the modified functions get called only once when
following a symlink or creating a hardlink.

The patch can be also downloaded from:
http://pearls.tuxedo-es.org/patches/linking-protections-2...

Cheers,
-- 
Lorenzo Hernández García-Hierro <lorenzo@gnu.org> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]


diff -Nur linux-2.6.11-rc3/fs/namei.c linux-2.6.11-rc3.slink/fs/namei.c
--- linux-2.6.11-rc3/fs/namei.c	2005-02-06 21:40:41.000000000 +0100
+++ linux-2.6.11-rc3.slink/fs/namei.c	2005-02-07 19:15:22.690689272 +0100
@@ -519,6 +519,19 @@
 	err = security_inode_follow_link(dentry, nd);
 	if (err)
 		goto loop;
+
+	/* Prevent users to follow symlinks owned by other users in
+	 * world-writable +t directories, unless the owner of the
+	 * symlink is the owner of the directory.
+	 */
+	if (S_ISLNK(dentry->d_inode->i_mode) &&
+	    (dentry->d_parent->d_inode->i_mode & S_ISVTX) 
+	    && (dentry->d_parent->d_inode->i_uid != dentry->d_inode->i_uid) &&
+	    (dentry->d_parent->d_inode->i_mode & S_IWOTH) && (current->fsuid != dentry->d_inode->i_uid))
{
+		err = -EACCES;
+		goto loop;
+	}
+			
 	current->link_count++;
 	current->total_link_count++;
 	nd->depth++;
@@ -1985,7 +1998,22 @@
 	new_dentry = lookup_create(&nd, 0);
 	error = PTR_ERR(new_dentry);
 	if (!IS_ERR(new_dentry)) {
-		error = vfs_link(old_nd.dentry, nd.dentry->d_inode, new_dentry);
+		error = 0;
+		
+		/* Check that the user who is trying to make the hardlink owns
+		 * the target file being linked (DAC->@old_nd.dentry->d_inode) */
+		if (current->fsuid != old_nd.dentry->d_inode->i_uid && 
+		(!S_ISREG(old_nd.dentry->d_inode->i_mode) || (old_nd.dentry->d_inode->i_mode & S_ISUID) ||
+		((old_nd.dentry->d_inode->i_mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
||
+	     	(generic_permission(old_nd.dentry->d_inode, MAY_READ | MAY_WRITE, NULL)))
&&
+	    	!capable(CAP_FOWNER) && current->uid) {
+		error = -EPERM;
+		}
+		
+		/* If @error is empty, then we apply the *normal* behavior */
+		if (!error)
+			error = vfs_link(old_nd.dentry, nd.dentry->d_inode, new_dentry);
+		
 		dput(new_dentry);
 	}
 	up(&nd.dentry->d_inode->i_sem);

Copyright © 2005, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds