From 2011181cbe60b256ced8d28daf7b704e8613467c Mon Sep 17 00:00:00 2001 From: John Wolfe Date: Wed, 18 Oct 2023 09:11:54 -0700 Subject: [PATCH] Address CVE-2023-34059 Fix file descriptor vulnerability in the open-vm-tools vmware-user-suid-wrapper on Linux. - 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. --- open-vm-tools/services/vmtoolsd/mainPosix.c | 76 +++++++++++++++++++++++++++ open-vm-tools/vmware-user-suid-wrapper/main.c | 26 ++------- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/open-vm-tools/services/vmtoolsd/mainPosix.c b/open-vm-tools/services/vmtoolsd/mainPosix.c index fd2667c..8b46979 100644 --- a/open-vm-tools/services/vmtoolsd/mainPosix.c +++ b/open-vm-tools/services/vmtoolsd/mainPosix.c @@ -28,10 +28,12 @@ #include #include #include +#include #include #include "file.h" #include "guestApp.h" #include "hostinfo.h" +#include "su.h" #include "system.h" #include "unicode.h" #include "util.h" @@ -155,6 +157,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. * * @param[in] argc Argument count. @@ -210,6 +265,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 e9d7e50..a19af53 100644 --- a/open-vm-tools/vmware-user-suid-wrapper/main.c +++ b/open-vm-tools/vmware-user-suid-wrapper/main.c @@ -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.6.2