Blob Blame History Raw
From e79672890799f6ed4dcfb44a4346489c9f3ea6e3 Mon Sep 17 00:00:00 2001
From: Katy Feng <fkaty@vmware.com>
Date: Tue, 17 Oct 2023 15:24:48 -0700
Subject: [PATCH 2/2] File descriptor vulnerability in the open-vm-tools
 vmware-user-suid-wrapperx

RH-Author: Ani Sinha <None>
RH-MergeRequest: 52: File descriptor vulnerability in the open-vm-tools vmware-user-suid-wrapperx on Linux
RH-Jira: RHEL-14676
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
RH-Commit: [1/1] c78a1d638849f1589b2f4b8ff1437b0f25bbd19c

Moving the privilege drop logic (dropping privilege to the real uid and
gid of the process for the vmusr service) from suidWrapper to vmtoolsd code.
Now the vmtoolsd is not executed with dropped privileges (started as setuid
program) and the dumpable attribute of the process is not reset.
The unprivileged user will not have access to the privileged file descriptors
in the vmtoolsd vmusr process.
Also, setting the FD_CLOEXEC flag for both uinputFd and blockFd preventing
the file descriptors being inherited any further from the vmtoolsd.

CVE-2023-34059
(cherry picked from commit 63f7c79c4aecb14d37cc4ce9da509419e31d394f)
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 open-vm-tools/services/vmtoolsd/mainPosix.c   | 78 ++++++++++++++++++-
 open-vm-tools/vmware-user-suid-wrapper/main.c | 28 +------
 2 files changed, 81 insertions(+), 25 deletions(-)

diff --git a/open-vm-tools/services/vmtoolsd/mainPosix.c b/open-vm-tools/services/vmtoolsd/mainPosix.c
index cfe67984..674d5bda 100644
--- a/open-vm-tools/services/vmtoolsd/mainPosix.c
+++ b/open-vm-tools/services/vmtoolsd/mainPosix.c
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2008-2019 VMware, Inc. All rights reserved.
+ * Copyright (c) 2008-2020,2022-2023 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -28,10 +28,12 @@
 #include <signal.h>
 #include <string.h>
 #include <unistd.h>
+#include <fcntl.h>
 #include <glib/gstdio.h>
 #include "file.h"
 #include "guestApp.h"
 #include "hostinfo.h"
+#include "su.h"
 #include "system.h"
 #include "unicode.h"
 #include "util.h"
@@ -153,6 +155,59 @@ ToolsCoreWorkAroundLoop(ToolsServiceState *state,
 }
 
 
+/**
+ * Tools function to set close-on-exec flg for the fd.
+ *
+ * @param[in] fd   open file descriptor.
+ *
+ * @return TRUE on success, FALSE otherwise.
+ */
+
+static gboolean
+ToolsSetCloexecFlag(int fd)
+{
+   int flags;
+
+   if (fd == -1) {
+      /* fd is not present, no need to manipulate */
+      return TRUE;
+   }
+
+   flags = fcntl(fd, F_GETFD, 0);
+   if (flags < 0) {
+      g_printerr("Couldn't get the flags set for fd %d, error %u.", fd, errno);
+      return FALSE;
+   }
+   flags |= FD_CLOEXEC;
+   if (fcntl(fd, F_SETFD, flags) < 0) {
+      g_printerr("Couldn't set close-on-exec for fd %d, error %u.", fd, errno);
+      return FALSE;
+   }
+
+   return TRUE;
+}
+
+
+/**
+ * Tools function to close the fds.
+ */
+
+static void
+ToolsCloseFds(void)
+{
+   if (gState.ctx.blockFD != -1) {
+      close(gState.ctx.blockFD);
+   }
+
+   /*
+    * uinputFD will be available only for wayland.
+    */
+   if (gState.ctx.uinputFD != -1) {
+      close(gState.ctx.uinputFD);
+   }
+}
+
+
 /**
  * Tools daemon entry function.
  *
@@ -200,6 +255,27 @@ main(int argc,
    g_free(argvCopy);
    argvCopy = NULL;
 
+   /*
+    * Drops privilege to the real uid and gid of the process
+    * for the "vmusr" service.
+    */
+   if (TOOLS_IS_USER_SERVICE(&gState)) {
+      uid_t uid = getuid();
+      gid_t gid = getgid();
+
+      if ((Id_SetREUid(uid, uid) != 0) ||
+          (Id_SetREGid(gid, gid) != 0)) {
+         g_printerr("could not drop privileges: %s", strerror(errno));
+         ToolsCloseFds();
+         goto exit;
+      }
+      if (!ToolsSetCloexecFlag(gState.ctx.blockFD) ||
+          !ToolsSetCloexecFlag(gState.ctx.uinputFD)) {
+         ToolsCloseFds();
+         goto exit;
+      }
+   }
+
    if (gState.pidFile != NULL) {
       /*
        * If argv[0] is not an absolute path, make it so; all other path
diff --git a/open-vm-tools/vmware-user-suid-wrapper/main.c b/open-vm-tools/vmware-user-suid-wrapper/main.c
index e9d7e508..73ae9b9b 100644
--- a/open-vm-tools/vmware-user-suid-wrapper/main.c
+++ b/open-vm-tools/vmware-user-suid-wrapper/main.c
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2007-2018 VMware, Inc. All rights reserved.
+ * Copyright (C) 2007-2018,2023 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -156,8 +156,7 @@ MaskSignals(void)
  *
  *    Obtains the library directory from the Tools locations database, then
  *    opens a file descriptor (while still root) to add and remove blocks,
- *    drops privilege to the real uid of this process, and finally starts
- *    vmware-user.
+ *    and finally starts vmware-user.
  *
  * Results:
  *    Parent: TRUE on success, FALSE on failure.
@@ -173,8 +172,6 @@ static Bool
 StartVMwareUser(char *const envp[])
 {
    pid_t pid;
-   uid_t uid;
-   gid_t gid;
    int blockFd = -1;
    char blockFdStr[8];
    int uinputFd = -1;
@@ -191,8 +188,8 @@ StartVMwareUser(char *const envp[])
    }
 
    /*
-    * Now create a child process, obtain a file descriptor as root, downgrade
-    * privilege, and run vmware-user.
+    * Now create a child process, obtain a file descriptor as root and
+    * run vmware-user.
     */
    pid = fork();
    if (pid == -1) {
@@ -229,23 +226,6 @@ StartVMwareUser(char *const envp[])
       }
    }
 
-   uid = getuid();
-   gid = getgid();
-
-   if ((setreuid(uid, uid) != 0) ||
-       (setregid(gid, gid) != 0)) {
-      Error("could not drop privileges: %s\n", strerror(errno));
-      if (blockFd != -1) {
-         close(blockFd);
-      }
-      if (useWayland) {
-         if (uinputFd != -1) {
-            close(uinputFd);
-         }
-      }
-      return FALSE;
-   }
-
    /*
     * Since vmware-user provides features that don't depend on vmblock, we
     * invoke vmware-user even if we couldn't obtain a file descriptor or we
-- 
2.39.3