| From 746e07f2d54908296dde64e97e12ea33a35063e0 Mon Sep 17 00:00:00 2001 |
| From: Vivek Goyal <vgoyal@redhat.com> |
| Date: Tue, 25 Jan 2022 13:51:14 -0500 |
| Subject: [PATCH] virtiofsd: Drop membership of all supplementary groups |
| (CVE-2022-0358) |
| |
| RH-Author: Dr. David Alan Gilbert <dgilbert@redhat.com> |
| RH-MergeRequest: 106: 8.5.0z non-av; virtiofsd security fix - drop secondary groups |
| RH-Commit: [1/1] e39df0b31f3c236675262395b94d5c10e8e3073f |
| RH-Bugzilla: 2048627 |
| RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com> |
| RH-Acked-by: Laszlo Ersek <lersek@redhat.com> |
| RH-Acked-by: Vivek Goyal <None> |
| |
| At the start, drop membership of all supplementary groups. This is |
| not required. |
| |
| If we have membership of "root" supplementary group and when we switch |
| uid/gid using setresuid/setsgid, we still retain membership of existing |
| supplemntary groups. And that can allow some operations which are not |
| normally allowed. |
| |
| For example, if root in guest creates a dir as follows. |
| |
| $ mkdir -m 03777 test_dir |
| |
| This sets SGID on dir as well as allows unprivileged users to write into |
| this dir. |
| |
| And now as unprivileged user open file as follows. |
| |
| $ su test |
| $ fd = open("test_dir/priviledge_id", O_RDWR|O_CREAT|O_EXCL, 02755); |
| |
| This will create SGID set executable in test_dir/. |
| |
| And that's a problem because now an unpriviliged user can execute it, |
| get egid=0 and get access to resources owned by "root" group. This is |
| privilege escalation. |
| |
| Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2044863 |
| Fixes: CVE-2022-0358 |
| Reported-by: JIETAO XIAO <shawtao1125@gmail.com> |
| Suggested-by: Miklos Szeredi <mszeredi@redhat.com> |
| Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
| Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> |
| Signed-off-by: Vivek Goyal <vgoyal@redhat.com> |
| Message-Id: <YfBGoriS38eBQrAb@redhat.com> |
| Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> |
| dgilbert: Fixed missing {}'s style nit |
| (cherry picked from commit 449e8171f96a6a944d1f3b7d3627ae059eae21ca) |
| dgilbert: Minor fixup around #includes on backport |
| |
| tools/virtiofsd/passthrough_ll.c | 27 +++++++++++++++++++++++++++ |
| 1 file changed, 27 insertions(+) |
| |
| diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c |
| index b47029da89..578131179c 100644 |
| |
| |
| @@ -63,6 +63,7 @@ |
| #include <sys/xattr.h> |
| #include <syslog.h> |
| #include <unistd.h> |
| +#include <grp.h> |
| |
| #include "passthrough_helpers.h" |
| #include "seccomp.h" |
| @@ -1058,6 +1059,30 @@ static void lo_lookup(fuse_req_t req, fuse_ino_t parent, const char *name) |
| #define OURSYS_setresuid SYS_setresuid |
| #endif |
| |
| +static void drop_supplementary_groups(void) |
| +{ |
| + int ret; |
| + |
| + ret = getgroups(0, NULL); |
| + if (ret == -1) { |
| + fuse_log(FUSE_LOG_ERR, "getgroups() failed with error=%d:%s\n", |
| + errno, strerror(errno)); |
| + exit(1); |
| + } |
| + |
| + if (!ret) { |
| + return; |
| + } |
| + |
| + /* Drop all supplementary groups. We should not need it */ |
| + ret = setgroups(0, NULL); |
| + if (ret == -1) { |
| + fuse_log(FUSE_LOG_ERR, "setgroups() failed with error=%d:%s\n", |
| + errno, strerror(errno)); |
| + exit(1); |
| + } |
| +} |
| + |
| /* |
| * Change to uid/gid of caller so that file is created with |
| * ownership of caller. |
| @@ -3010,6 +3035,8 @@ int main(int argc, char *argv[]) |
| /* Don't mask creation mode, kernel already did that */ |
| umask(0); |
| |
| + drop_supplementary_groups(); |
| + |
| pthread_mutex_init(&lo.mutex, NULL); |
| lo.inodes = g_hash_table_new(lo_key_hash, lo_key_equal); |
| lo.root.fd = -1; |
| -- |
| 2.27.0 |
| |