d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fusefs: Fix another obscure permission handling bug

Browse Source 
Don't allow unprivileged users to set SGID on files to whose group they
don't belong.  This is slightly different than what POSIX says we should do
(clear sgid on return from a successful chmod), but it matches what UFS
currently does.

Reported by:	pjdfstest
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-05-06 16:54:35 +00:00
parent a90e32de25
commit 8cfb44315a
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/projects/fuse2/; revision=347191
2 changed files with 60 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
sys/fs/fuse/fuse_vnops.c
View File

@ -1589,6 +1589,17 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
if (checkperm && vp->v_type != VDIR && (vap->va_mode & S_ISTXT)
&& priv_check_cred(cred, PRIV_VFS_STICKYFILE))
return EFTYPE;
if (checkperm && (vap->va_mode & S_ISGID)) {
struct vattr old_va;
err = fuse_internal_getattr(vp, &old_va, cred, td);
if (err)
return (err);
if (!groupmember(old_va.va_gid, cred)) {
err = priv_check_cred(cred, PRIV_VFS_SETGID);
if (err)
return (err);
}
}
accmode |= VADMIN;
}

64
tests/sys/fs/fusefs/default_permissions.cc
View File

@ -225,6 +225,27 @@ void expect_setxattr(int error)
}
};
/* Return a group to which this user does not belong */
static gid_t excluded_group()
{
int i, ngroups = 64;
gid_t newgid, groups[ngroups];
getgrouplist(getlogin(), getegid(), groups, &ngroups);
for (newgid = 0; newgid >= 0; newgid++) {
bool belongs = false;
for (i = 0; i < ngroups; i++) {
if (groups[i] == newgid)
belongs = true;
}
if (!belongs)
break;
}
/* newgid is now a group to which the current user does not belong */
return newgid;
}
TEST_F(Access, eacces)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
@ -304,26 +325,12 @@ TEST_F(Chgrp, eperm)
const char RELPATH[] = "some_file.txt";
const uint64_t ino = 42;
const mode_t mode = 0755;
int ngroups = 64;
gid_t groups[ngroups];
uid_t uid;
gid_t gid, newgid;
int i;
uid = geteuid();
gid = getegid();
getgrouplist(getlogin(), getegid(), groups, &ngroups);
for (newgid = 0; newgid >= 0; newgid++) {
bool belongs = false;
for (i = 0; i < ngroups; i++) {
if (groups[i] == newgid)
belongs = true;
}
if (!belongs)
break;
}
/* newgid is now a group to which the current user does not belong */
newgid = excluded_group();
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid, gid);
expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, uid, gid);
@ -790,6 +797,33 @@ TEST_F(Setattr, eacces)
EXPECT_EQ(EPERM, errno);
}
/*
* Setting the sgid bit should fail for an unprivileged user who doesn't belong
* to the file's group
*/
TEST_F(Setattr, sgid_by_non_group_member)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
const uint64_t ino = 42;
const mode_t oldmode = 0755;
const mode_t newmode = 02755;
uid_t uid = geteuid();
gid_t gid = excluded_group();
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX, uid, gid);
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).Times(0);
EXPECT_NE(0, chmod(FULLPATH, newmode));
EXPECT_EQ(EPERM, errno);
}
/* Only the superuser may set the sticky bit on a non-directory */
TEST_F(Setattr, sticky_regular_file)
{