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