davezarzycki created this revision.
davezarzycki added reviewers: compnerd, aprantl, jakehehrlich, espindola, 
respindola, ilya-biryukov, pcc, sammccall.
davezarzycki added a project: LLVM.
Herald added subscribers: cfe-commits, hiraditya.
Herald added a project: clang.

Naive usage of shared temporary directories in Unix is a classic security 
problem. Let's deprecate two APIs that enable unsafe code and encourage people 
to use `createUniqueFile` or `createUniqueDirectory` instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82259

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp

Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -164,6 +164,41 @@
   FS_Name
 };
 
+// Avoid a deprecation warning by moving the createUniquePath body here.
+static void _createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
+                              bool MakeAbsolute) {
+  SmallString<128> ModelStorage;
+  Model.toVector(ModelStorage);
+
+  if (MakeAbsolute) {
+    // Make model absolute by prepending a temp directory if it's not already.
+    if (!sys::path::is_absolute(Twine(ModelStorage))) {
+      SmallString<128> TDir;
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#endif
+      sys::path::system_temp_directory(true, TDir);
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+      sys::path::append(TDir, Twine(ModelStorage));
+      ModelStorage.swap(TDir);
+    }
+  }
+
+  ResultPath = ModelStorage;
+  ResultPath.push_back(0);
+  ResultPath.pop_back();
+
+  // Replace '%' with random chars.
+  for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
+    if (ModelStorage[i] == '%')
+      ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
+  }
+}
+
+
 static std::error_code
 createUniqueEntity(const Twine &Model, int &ResultFD,
                    SmallVectorImpl<char> &ResultPath, bool MakeAbsolute,
@@ -176,7 +211,7 @@
   // Checking which is racy, so we try a number of times, then give up.
   std::error_code EC;
   for (int Retries = 128; Retries > 0; --Retries) {
-    sys::fs::createUniquePath(Model, ResultPath, MakeAbsolute);
+    _createUniquePath(Model, ResultPath, MakeAbsolute);
     // Try to open + create the file.
     switch (Type) {
     case FS_File: {
@@ -777,29 +812,8 @@
 }
 
 void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
-                      bool MakeAbsolute) {
-  SmallString<128> ModelStorage;
-  Model.toVector(ModelStorage);
-
-  if (MakeAbsolute) {
-    // Make model absolute by prepending a temp directory if it's not already.
-    if (!sys::path::is_absolute(Twine(ModelStorage))) {
-      SmallString<128> TDir;
-      sys::path::system_temp_directory(true, TDir);
-      sys::path::append(TDir, Twine(ModelStorage));
-      ModelStorage.swap(TDir);
-    }
-  }
-
-  ResultPath = ModelStorage;
-  ResultPath.push_back(0);
-  ResultPath.pop_back();
-
-  // Replace '%' with random chars.
-  for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
-    if (ModelStorage[i] == '%')
-      ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
-  }
+                              bool MakeAbsolute) {
+  return _createUniquePath(Model, ResultPath, MakeAbsolute);
 }
 
 std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
Index: llvm/include/llvm/Support/Path.h
===================================================================
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -363,7 +363,10 @@
 /// (e.g., TEMP on Windows, TMPDIR on *nix) to specify a temporary directory.
 ///
 /// @param result Holds the resulting path name.
-void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result);
+LLVM_ATTRIBUTE_DEPRECATED(
+void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result),
+  "Only meant for debugging due to trivial security problems"
+);
 
 /// Get the user's home directory.
 ///
Index: llvm/include/llvm/Support/FileSystem.h
===================================================================
--- llvm/include/llvm/Support/FileSystem.h
+++ llvm/include/llvm/Support/FileSystem.h
@@ -800,8 +800,11 @@
 /// @param Model Name to base unique path off of.
 /// @param ResultPath Set to the file's path.
 /// @param MakeAbsolute Whether to use the system temp directory.
-void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
-                      bool MakeAbsolute);
+LLVM_ATTRIBUTE_DEPRECATED(
+  void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
+                        bool MakeAbsolute),
+  "Use createUniqueFile() or createUniqueDirectory()"
+);
 
 /// Create a uniquely named file.
 ///
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -733,38 +733,6 @@
   }
 }
 
-static void appendUserToPath(SmallVectorImpl<char> &Result) {
-#ifdef LLVM_ON_UNIX
-  const char *Username = getenv("LOGNAME");
-#else
-  const char *Username = getenv("USERNAME");
-#endif
-  if (Username) {
-    // Validate that LoginName can be used in a path, and get its length.
-    size_t Len = 0;
-    for (const char *P = Username; *P; ++P, ++Len) {
-      if (!clang::isAlphanumeric(*P) && *P != '_') {
-        Username = nullptr;
-        break;
-      }
-    }
-
-    if (Username && Len > 0) {
-      Result.append(Username, Username + Len);
-      return;
-    }
-  }
-
-// Fallback to user id.
-#ifdef LLVM_ON_UNIX
-  std::string UID = llvm::utostr(getuid());
-#else
-  // FIXME: Windows seems to have an 'SID' that might work.
-  std::string UID = "9999";
-#endif
-  Result.append(UID.begin(), UID.end());
-}
-
 static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
                                    const Driver &D, const InputInfo &Output,
                                    const ArgList &Args,
@@ -3203,9 +3171,8 @@
 }
 
 void Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
-  llvm::sys::path::append(Result, "org.llvm.clang.");
-  appendUserToPath(Result);
+  llvm::sys::path::home_directory(Result);
+  llvm::sys::path::append(Result, ".org.llvm.clang");
   llvm::sys::path::append(Result, "ModuleCache");
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to