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

--- a/tools/virtiofsd/passthrough_ll.c

+++ b/tools/virtiofsd/passthrough_ll.c

@@ -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