Date: Mon, 6 May 2019 16:54:36 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r347191 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201905061654.x46Gsaa0030023@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Mon May 6 16:54:35 2019 New Revision: 347191 URL: https://svnweb.freebsd.org/changeset/base/347191 Log: fusefs: Fix another obscure permission handling bug 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 Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Mon May 6 16:22:45 2019 (r347190) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Mon May 6 16:54:35 2019 (r347191) @@ -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; } Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Mon May 6 16:22:45 2019 (r347190) +++ projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Mon May 6 16:54:35 2019 (r347191) @@ -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,27 +325,13 @@ 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; + newgid = excluded_group(); - 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 */ - expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid, gid); expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, uid, gid); EXPECT_CALL(*m_mock, process( @@ -779,6 +786,33 @@ TEST_F(Setattr, eacces) expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1); expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX, 0); + 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); +} + +/* + * 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905061654.x46Gsaa0030023>