kern/116515: NTFS mount does not check that user has permissions on the device

[Patrick Lamaiziere]

>Number:         116515
>Category:       kern
>Synopsis:       NTFS mount does not check that user has permissions on the device
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Sep 21 16:00:05 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Patrick Lamaiziere
>Release:        6.2-STABLE/i386
>Organization:
>Environment:
FreeBSD roxette.lamaiziere.net 6.2-STABLE FreeBSD 6.2-STABLE #2: Fri Sep 14 00:29:52 CEST 2007     patrick at roxette.lamaiziere.net:/usr/obj/usr/src/sys/GENERIC  i386
>Description:
The NTFS file system does not check that the user has necessary
permissions on the device to mount it when vfs.usermount is set to "1". 

This problem allows any user to mount (and then to use) any ntfs file system, without any permission on the device. But only if vfs.usermount is set to "1".
I think this is a security issue...

In their vfs operation "mount", others file systems (ffs, msdosfs, udf, ext,...) check that user has necessary permissions on the device with a test.
But this test is missing in the NTFS file system.

The test looks like (see by example : sys/fs/msdosfs/msdosfs_vfsops.c at line 357.)

/*
 * If mount by non-root, then verify that user has necessary
 * permissions on the device.
 */
if (suser(td)) {
   accessmode = VREAD;
   if ((mp->mnt_flag & MNT_RDONLY) == 0)
     accessmode |= VWRITE;
     if ((error = VOP_ACCESS(devvp, accessmode, td->td_ucred, td))!= 0){
       vput(devvp);
       return (error);
     }
}

>How-To-Repeat:
Sample (/dev/ad0s2 is my NTFS/MS-Windows slice)
I am the user "toto".

$ sysctl vfs.usermount
vfs.usermount: 1
$ id
uid=1002(toto) gid=1004(toto) groups=1004(toto)
$ ls -l /dev/ad0s2
crw-r-----  1 root  operator    0,  96 12 sep 21:52 /dev/ad0s2

(user "toto" is not in the group operator and should not be allowed to mount the device)

$ mkdir /usr/home/toto/win
$ mount_ntfs /dev/ad0s2 /usr/home/toto/win
$ mount
/dev/ad0s3a on / (ufs, local)
devfs on /dev (devfs, local)
/dev/ad0s3e on /tmp (ufs, local, soft-updates)
/dev/ad0s3f on /usr (ufs, local, soft-updates)
/dev/ad0s3d on /var (ufs, local, soft-updates)
/dev/ad0s2 on /usr/home/toto/win (ntfs, local, nosuid, mounted by toto)

$ ls /usr/home/toto/win
$AttrDef*                       RECYCLER/
[...]
>Fix:
Verify in the mount operation of the NTFS file system that user has necessary permissions. File sys/fs/ntfs/ntfs_vfsops.c, function ntfs_mount()
 
The attached patch checks the permission. This is a merge with the file sys/fs/msdosfs/msdosfs_vfsops.c. 
It seems to work but i'm not sure if this is the good way to do this check.


Patch attached with submission follows:

--- sys/fs/ntfs/ntfs_vfsops.org	2007-09-14 00:13:35.000000000 +0200
+++ sys/fs/ntfs/ntfs_vfsops.c	2007-09-14 00:19:46.000000000 +0200
@@ -157,6 +157,7 @@ ntfs_mount ( 
 	struct vnode	*devvp;
 	struct nameidata ndp;
 	char *from;
+	mode_t accessmode;
 
 	if (vfs_filteropt(mp->mnt_optnew, ntfs_opts))
 		return (EINVAL);
@@ -198,6 +199,20 @@ ntfs_mount ( 
 		return (err);
 	}
 
+	/*
+	 * If mount by non-root, then verify that user has necessary
+	 * permissions on the device.
+	 */
+	if (suser(td)) {
+		accessmode = VREAD;
+		if ((mp->mnt_flag & MNT_RDONLY) == 0)
+			accessmode |= VWRITE;
+		if ((error = VOP_ACCESS(devvp, accessmode, td->td_ucred, td))!= 0){
+			vput(devvp);
+			return (error);
+		}
+	}
+
 	if (mp->mnt_flag & MNT_UPDATE) {
 #if 0
 		/*


>Release-Note:
>Audit-Trail:
>Unformatted: