svn commit: r347236 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs

[Alan Somers]

Author: asomers
Date: Tue May  7 22:38:13 2019
New Revision: 347236
URL: https://svnweb.freebsd.org/changeset/base/347236

Log:
  fusefs: drop suid after a successful chown by a non-root user
  
  Drop sgid too.  Also, drop them after a successful chgrp.
  
  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	Tue May  7 21:15:11 2019	(r347235)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c	Tue May  7 22:38:13 2019	(r347236)
@@ -1524,6 +1524,7 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 	int err = 0, err2;
 	accmode_t accmode = 0;
 	bool checkperm;
+	bool drop_suid = false;
 	gid_t cr_gid;
 
 	mp = vnode_mount(vp);
@@ -1553,12 +1554,15 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 					return err;
 				else
 					accmode |= VADMIN;
+				drop_suid = true;
 			} else
 				accmode |= VADMIN;
 		} else
 			accmode |= VADMIN;
 	}
 	if (vap->va_gid != (gid_t)VNOVAL) {
+		if (checkperm && priv_check_cred(cred, PRIV_VFS_CHOWN))
+			drop_suid = true;
 		if (checkperm && !groupmember(vap->va_gid, cred))
 		{
 			/*
@@ -1574,8 +1578,7 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 					return (err2);
 				if (vap->va_gid != old_va.va_gid)
 					return err;
-				else
-					accmode |= VADMIN;
+				accmode |= VADMIN;
 			} else
 				accmode |= VADMIN;
 		} else
@@ -1611,6 +1614,16 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 		accmode |= VADMIN;
 	if (vap->va_mtime.tv_sec != VNOVAL)
 		accmode |= VADMIN;
+	if (drop_suid) {
+		if (vap->va_mode != (mode_t)VNOVAL)
+			vap->va_mode &= ~(S_ISUID | S_ISGID);
+		else {
+			err = fuse_internal_getattr(vp, &old_va, cred, td);
+			if (err)
+				return (err);
+			vap->va_mode = old_va.va_mode & ~(S_ISUID | S_ISGID);
+		}
+	}
 	if (vap->va_mode != (mode_t)VNOVAL) {
 		/* Only root may set the sticky bit on non-directories */
 		if (checkperm && vp->v_type != VDIR && (vap->va_mode & S_ISTXT)

Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc	Tue May  7 21:15:11 2019	(r347235)
+++ projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc	Tue May  7 22:38:13 2019	(r347236)
@@ -322,6 +322,41 @@ TEST_F(Chown, chown_to_self)
 	EXPECT_EQ(0, chown(FULLPATH, uid, -1)) << strerror(errno);
 }
 
+/*
+ * A successful chown by a non-privileged non-owner should clear a file's SUID
+ * bit
+ */
+TEST_F(Chown, clear_suid)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	const mode_t oldmode = 06755;
+	const mode_t newmode = 0755;
+	uid_t uid = geteuid();
+	uint32_t valid = FATTR_UID | FATTR_MODE;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid);
+	expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX, uid);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_SETATTR &&
+				in->header.nodeid == ino &&
+				in->body.setattr.valid == valid &&
+				in->body.setattr.mode == newmode);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out->body.attr.attr.ino = ino;	// Must match nodeid
+		out->body.attr.attr.mode = S_IFREG | newmode;
+		out->body.attr.attr_valid = UINT64_MAX;
+	})));
+
+	EXPECT_EQ(0, chown(FULLPATH, uid, -1)) << strerror(errno);
+}
+
+
 /* Only root may change a file's owner */
 TEST_F(Chown, eperm)
 {
@@ -341,6 +376,41 @@ TEST_F(Chown, eperm)
 
 	EXPECT_NE(0, chown(FULLPATH, 0, -1));
 	EXPECT_EQ(EPERM, errno);
+}
+
+/*
+ * A successful chgrp by a non-privileged non-owner should clear a file's SUID
+ * bit
+ */
+TEST_F(Chgrp, clear_suid)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	const mode_t oldmode = 06755;
+	const mode_t newmode = 0755;
+	uid_t uid = geteuid();
+	gid_t gid = getegid();
+	uint32_t valid = FATTR_GID | FATTR_MODE;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid);
+	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 &&
+				in->header.nodeid == ino &&
+				in->body.setattr.valid == valid &&
+				in->body.setattr.mode == newmode);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out->body.attr.attr.ino = ino;	// Must match nodeid
+		out->body.attr.attr.mode = S_IFREG | newmode;
+		out->body.attr.attr_valid = UINT64_MAX;
+	})));
+
+	EXPECT_EQ(0, chown(FULLPATH, -1, gid)) << strerror(errno);
 }
 
 /* non-root users may only chgrp a file to a group they belong to */