From: Oliver Kurth <okurth@vmware.com>
Date: Mon, 26 Feb 2018 20:29:07 +0000 (-0800)
Subject: lib/file: File_GetSafeTmpDir is not aware of credentials changes
X-Git-Tag: stable-10.3.0~104
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d244dac08f07cbbcba3f66eab7d78c16150edfba;p=thirdparty%2Fopen-vm-tools.git

lib/file: File_GetSafeTmpDir is not aware of credentials changes

Some applications may masquerade their use (change their EUID).
Futhermore, each thread in an application can have a separate EUID.

Anytime a application or thread asked for its safe temporary directory,
it needs to get the same result regardless of how many times it asks.

Change File_GetSafeTmpDir to cache the EUID associated with the cached
values. If there is an EUID change, invalidate the existing cache
entries and recompute them. The recomputation process is stable, in
that it will obtain the same return any time it is called.

This way we get the benefit of the cache (performance), ensured
correctness (for applications that do not masquerade), and correctness
for those applications that do masquerade.
---

diff --git a/open-vm-tools/lib/file/fileTempPosix.c b/open-vm-tools/lib/file/fileTempPosix.c
index d035ae890..22c627bd3 100644
--- a/open-vm-tools/lib/file/fileTempPosix.c
+++ b/open-vm-tools/lib/file/fileTempPosix.c
@@ -470,9 +470,9 @@ FileCreateSafeTmpDir(const char *baseTmpDir,  // IN:
  *      is not prone to symlink attacks, because it is only writable with the
  *      current set of credentials (EUID).
  *
- *      Guaranteed to return the same directory every time it is called during
- *      the lifetime of the current process, for the current effective UID.
- *      (Barring the user manually deleting or renaming the directory.)
+ *      Guaranteed to return the same directory for any EUID every time it is
+ *      called during the lifetime of the current process. (Barring the user
+ *      manually deleting or renaming the directory.)
  *
  *      Optionally, add the PID to the user identifier for the cases where
  *      the EUID may change during the lifetime of the calling process.
@@ -497,13 +497,14 @@ FileGetSafeTmpDir(Bool useConf,  // IN: Use configuration variables?
    tmpDir = FileGetTmpDir(useConf);
 #else
    static Atomic_Ptr lckStorage;
-   static char *safeDir;
-   static char *safePidDir;
-   MXUserExclLock *lck;
+   static char *cachedDir;
+   static uid_t cachedEuid;
+   static char *cachedPidDir;
+   uid_t euid;
    char *testSafeDir;
+   MXUserExclLock *lck;
    char *userName = NULL;
    char *baseTmpDir = NULL;
-   uid_t euid = geteuid();
 
    /* Get and take lock for our safe dir. */
    lck = MXUser_CreateSingletonExclLock(&lckStorage, "getSafeTmpDirLock",
@@ -512,13 +513,22 @@ FileGetSafeTmpDir(Bool useConf,  // IN: Use configuration variables?
    MXUser_AcquireExclLock(lck);
 
    /*
-    * Based on the addPid boolean argument, check if we've created a
-    * temporary dir already and if it is still usable.
+    * If a suitable temporary directory was cached for this EUID, use it...
+    * as long as it is still acceptable.
     */
 
-   testSafeDir = addPid ? safePidDir : safeDir;
+   euid = geteuid();
 
-   if ((testSafeDir != NULL) && FileAcceptableSafeTmpDir(testSafeDir, euid)) {
+   testSafeDir = addPid ? cachedPidDir : cachedDir;
+
+   /*
+    * Detecting an EUID change without resorting to I/Os is an nice performance
+    * improvement... particularly on ESXi where the operations are expensive.
+    */
+
+   if ((euid == cachedEuid) &&
+       (testSafeDir != NULL) &&
+       FileAcceptableSafeTmpDir(testSafeDir, euid)) {
       tmpDir = Util_SafeStrdup(testSafeDir);
       goto exit;
    }
@@ -564,12 +574,14 @@ FileGetSafeTmpDir(Bool useConf,  // IN: Use configuration variables?
       testSafeDir = Util_SafeStrdup(tmpDir);
 
       if (addPid) {
-         Posix_Free(safePidDir);
-         safePidDir = testSafeDir;
+         Posix_Free(cachedPidDir);
+         cachedPidDir = testSafeDir;
       } else {
-         Posix_Free(safeDir);
-         safeDir = testSafeDir;
+         Posix_Free(cachedDir);
+         cachedDir = testSafeDir;
       }
+
+      cachedEuid = euid;
    }
 
 exit:
@@ -592,9 +604,9 @@ exit:
  *      is not prone to symlink attacks, because it is only writable with the
  *      current set of credentials (EUID).
  *
- *      Guaranteed to return the same directory every time it is called during
- *      the lifetime of the current process, for the current effective UID.
- *      (Barring the user manually deleting or renaming the directory.)
+ *      Guaranteed to return the same directory for any EUID every time it is
+ *      called during the lifetime of the current process. (Barring the user
+ *      manually deleting or renaming the directory.)
  *
  * Results:
  *     !NULL  Path to safe temp directory (must be freed)
@@ -622,9 +634,9 @@ File_GetSafeTmpDir(Bool useConf)  // IN:
  *      is not prone to symlink attacks, because it is only writable with the
  *      current set of credentials (EUID).
  *
- *      Guaranteed to return the same directory every time it is called during
- *      the lifetime of the current process, for the current effective UID.
- *      (Barring the user manually deleting or renaming the directory.)
+ *      Guaranteed to return the same directory for any EUID every time it is
+ *      called during the lifetime of the current process. (Barring the user
+ *      manually deleting or renaming the directory.)
  *
  * Results:
  *     !NULL  Path to safe temp directory (must be freed).