Date: Sun, 19 Oct 1997 20:02:42 -0700 (PDT) From: pavel@slac.stanford.edu To: FreeBSD-gnats-submit@FreeBSD.ORG Subject: kern/4810: Access checks in msdosfs_mount() Message-ID: <199710200302.UAA01419@bozon.Stanford.EDU> Resent-Message-ID: <199710200310.UAA21189@hub.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 4810
>Category: kern
>Synopsis: Access checks in msdosfs_mount()
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun Oct 19 20:10:01 PDT 1997
>Last-Modified:
>Originator: Tom Pavel
>Organization:
Stanford Linear Accelerator Center
>Release: FreeBSD 2.2-STABLE i386
>Environment:
>Description:
I noticed this problem because I use a setuid wrapper to allow
non-privileged users to mount /def/fd0 as /floppy. I have noticed that
this wrapper works fine for the cd9660 fs (/dev/wcd0c on /cdrom), but
fails for msdos fs unless the setuid wrapper is executed by root.
I have now traced to problem to the following code in msdosfs_vfsops.c:
------------------------------------------------------
/*
* check to see that the user in owns the target directory.
* Note the very XXX trick to make sure we're checking as the
* real user -- were mount() executable by anyone, this wouldn't
* be a problem.
*
* XXX there should be one consistent error out.
*/
cred = crdup(p->p_ucred); /* XXX */
cred->cr_uid = p->p_cred->p_ruid; /* XXX */
error = VOP_GETATTR(mp->mnt_vnodecovered, &va, cred, p);
if (error) {
crfree(cred); /* XXX */
return error;
}
if (cred->cr_uid != 0) {
if (va.va_uid != cred->cr_uid) {
error = EACCES;
crfree(cred); /* XXX */
return error;
}
/* a user mounted it; we'll verify permissions when unmounting */
mp->mnt_flag |= MNT_USER;
}
------------------------------------------------------
This code seems intent on checking only the real uid. Perhaps that
semantics is required for some part of the msdosfs security model, but
I find it incovenient and I can't see the justification. If the euid
of the process doing the mount is root, then I think the mount should
be allowed.
>How-To-Repeat:
>Fix:
Not sure of the best fix. Perhaps just change:
if (cred->cr_uid != 0) {
to:
if (p->p_ucred->cr_uid != 0 && p->p_cred->p_ruid != 0) {
Tom Pavel
Stanford Linear Accelerator Center
pavel@slac.stanford.edu
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199710200302.UAA01419>