virtiofsd: Drop membership of all supplementary groups (CVE-2022-0358)

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
This commit is contained in:
Vivek Goyal 2022-01-25 13:51:14 -05:00 committed by Dr. David Alan Gilbert
parent aeb0ae95b7
commit 449e8171f9

View File

@ -54,6 +54,7 @@
#include <sys/wait.h> #include <sys/wait.h>
#include <sys/xattr.h> #include <sys/xattr.h>
#include <syslog.h> #include <syslog.h>
#include <grp.h>
#include "qemu/cutils.h" #include "qemu/cutils.h"
#include "passthrough_helpers.h" #include "passthrough_helpers.h"
@ -1161,6 +1162,30 @@ static void lo_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
#define OURSYS_setresuid SYS_setresuid #define OURSYS_setresuid SYS_setresuid
#endif #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 * Change to uid/gid of caller so that file is created with
* ownership of caller. * ownership of caller.
@ -3926,6 +3951,8 @@ int main(int argc, char *argv[])
qemu_init_exec_dir(argv[0]); qemu_init_exec_dir(argv[0]);
drop_supplementary_groups();
pthread_mutex_init(&lo.mutex, NULL); pthread_mutex_init(&lo.mutex, NULL);
lo.inodes = g_hash_table_new(lo_key_hash, lo_key_equal); lo.inodes = g_hash_table_new(lo_key_hash, lo_key_equal);
lo.root.fd = -1; lo.root.fd = -1;