[PATCH] Filesystem linking protections

["Lorenzo Hernández García-Hierro" <lorenzo@gnu.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1325 bytes --]

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.6.11-rc3.patch

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


[-- Attachment #1.2: linking-protections-2.6.11-rc3.patch --]
[-- Type: text/x-patch, Size: 1864 bytes --]

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);

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 189 bytes --]