From d4fb8d71ef0d6785d6324b04a7efb00a89928ca8 Mon Sep 17 00:00:00 2001
From: asomers <asomers@FreeBSD.org>
Date: Mon, 6 May 2019 16:54:35 +0000
Subject: [PATCH] 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
---
 sys/fs/fuse/fuse_vnops.c                   | 11 ++++
 tests/sys/fs/fusefs/default_permissions.cc | 64 +++++++++++++++++-----
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index b30b0580e97..554167b42d2 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -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;
 	}
 
diff --git a/tests/sys/fs/fusefs/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc
index 17dc3c6b763..78809af24da 100644
--- a/tests/sys/fs/fusefs/default_permissions.cc
+++ b/tests/sys/fs/fusefs/default_permissions.cc
@@ -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)
 {
-- 
2.45.0