[PATCH] Filesystem linking protections

[Lorenzo Hern�ndez Garc�a-Hierro]

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 <[EMAIL PROTECTED]> 
[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);

signature.asc
Description: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada digitalmente