From b6d85b595ea7c9e0fca10d5e77a48102110fe22c Mon Sep 17 00:00:00 2001 From: Xiubo Li Date: Wed, 3 Apr 2024 19:02:08 +0800 Subject: [PATCH] client: disallow unprivileged users to escalate root privileges An unprivileged user can `chmod 777` a directory owned by root and gain access. Fix this bug and also add a test case for the same. Signed-off-by: Xiubo Li Signed-off-by: Venky Shankar --- src/client/Client.cc | 24 ++++++++++++++---------- src/test/libcephfs/suidsgid.cc | 10 ++++++++++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 1cb58c815f9e3..49ca4322bbb27 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -5897,18 +5897,22 @@ int Client::may_setattr(Inode *in, struct ceph_statx *stx, int mask, } if (mask & CEPH_SETATTR_MODE) { + bool allowed = false; + /* + * Currently the kernel fuse and libfuse code is buggy and + * won't pass the ATTR_KILL_SUID/ATTR_KILL_SGID to ceph-fuse. + * But will just set the ATTR_MODE and at the same time by + * clearing the suid/sgid bits. + * + * Only allow unprivileged users to clear S_ISUID and S_ISUID. + */ + if ((in->mode & (S_ISUID | S_ISGID)) != (stx->stx_mode & (S_ISUID | S_ISGID)) && + (in->mode & ~(S_ISUID | S_ISGID)) == (stx->stx_mode & ~(S_ISUID | S_ISGID))) { + allowed = true; + } uint32_t m = ~stx->stx_mode & in->mode; // mode bits removed ldout(cct, 20) << __func__ << " " << *in << " = " << hex << m << dec << dendl; - if (perms.uid() != 0 && perms.uid() != in->uid && - /* - * Currently the kernel fuse and libfuse code is buggy and - * won't pass the ATTR_KILL_SUID/ATTR_KILL_SGID to ceph-fuse. - * But will just set the ATTR_MODE and at the same time by - * clearing the suid/sgid bits. - * - * Only allow unprivileged users to clear S_ISUID and S_ISUID. - */ - (m & ~(S_ISUID | S_ISGID))) + if (perms.uid() != 0 && perms.uid() != in->uid && !allowed) goto out; gid_t i_gid = (mask & CEPH_SETATTR_GID) ? stx->stx_gid : in->gid; diff --git a/src/test/libcephfs/suidsgid.cc b/src/test/libcephfs/suidsgid.cc index d750613ebd814..474795cc455d4 100644 --- a/src/test/libcephfs/suidsgid.cc +++ b/src/test/libcephfs/suidsgid.cc @@ -134,6 +134,14 @@ void run_truncate_test_case(int mode, int result, size_t size, bool with_admin=f ceph_close(_cmount, fd); } +void run_change_mode_test_case() +{ + char c_dir[1024]; + sprintf(c_dir, "/mode_test_%d", getpid()); + ASSERT_EQ(0, ceph_mkdirs(admin, c_dir, 0700)); + ASSERT_EQ(ceph_chmod(cmount, c_dir, 0777), -CEPHFS_EPERM); +} + TEST(SuidsgidTest, WriteClearSetuid) { ASSERT_EQ(0, ceph_create(&admin, NULL)); ASSERT_EQ(0, ceph_conf_read_file(admin, NULL)); @@ -206,6 +214,8 @@ TEST(SuidsgidTest, WriteClearSetuid) { // 14, Truncate by unprivileged user clears the suid and sgid run_truncate_test_case(06766, 0, 100); + run_change_mode_test_case(); + // clean up ceph_shutdown(cmount); ceph_shutdown(admin); -- 2.39.5