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
This commit is contained in:
parent
4e83d6555e
commit
a2bdd7379b
@ -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)
|
||||
|
@ -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)
|
||||
{
|
||||
@ -343,6 +378,41 @@ TEST_F(Chown, eperm)
|
||||
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 */
|
||||
TEST_F(Chgrp, eperm)
|
||||
{
|
||||
|
Loading…
Reference in New Issue
Block a user