[libc++] Fix TOCTOU issue with std::filesystem::remove_all

https://bugs.chromium.org/p/llvm/issues/detail?id=19
rdar://87912416

Differential Revision: https://reviews.llvm.org/D118134

libcxx/src/filesystem/operations.cpp

@@ -24,9 +24,10 @@
 # define NOMINMAX
 # include <windows.h>
 #else
-# include <unistd.h>
+# include <dirent.h>
 # include <sys/stat.h>
 # include <sys/statvfs.h>
+# include <unistd.h>
 #endif
 #include <time.h>
 #include <fcntl.h> /* values for fchmodat */
@@ -1338,6 +1339,19 @@ bool __remove(const path& p, error_code* ec) {
   return true;
 }
 
+// We currently have two implementations of `__remove_all`. The first one is general and
+// used on platforms where we don't have access to the `openat()` family of POSIX functions.
+// That implementation uses `directory_iterator`, however it is vulnerable to some race
+// conditions, see https://reviews.llvm.org/D118134 for details.
+//
+// The second implementation is used on platforms where `openat()` & friends are available,
+// and it threads file descriptors through recursive calls to avoid such race conditions.
+#if defined(_LIBCPP_WIN32API)
+# define REMOVE_ALL_USE_DIRECTORY_ITERATOR
+#endif
+
+#if defined(REMOVE_ALL_USE_DIRECTORY_ITERATOR)
+
 namespace {
 
 uintmax_t remove_all_impl(path const& p, error_code& ec) {
@@ -1377,6 +1391,97 @@ uintmax_t __remove_all(const path& p, error_code* ec) {
   return count;
 }
 
+#else // !REMOVE_ALL_USE_DIRECTORY_ITERATOR
+
+namespace {
+
+template <class Cleanup>
+struct scope_exit {
+  explicit scope_exit(Cleanup const& cleanup)
+    : cleanup_(cleanup)
+  { }
+
+  ~scope_exit() { cleanup_(); }
+
+private:
+  Cleanup cleanup_;
+};
+
+uintmax_t remove_all_impl(int parent_directory, const path& p, error_code& ec) {
+  // First, try to open the path as a directory.
+  const int options = O_CLOEXEC | O_RDONLY | O_DIRECTORY | O_NOFOLLOW;
+  int fd = ::openat(parent_directory, p.c_str(), options);
+  if (fd != -1) {
+    // If that worked, iterate over the contents of the directory and
+    // remove everything in it, recursively.
+    scope_exit close_fd([=] { ::close(fd); });
+    DIR* stream = ::fdopendir(fd);
+    if (stream == nullptr) {
+      ec = detail::capture_errno();
+      return 0;
+    }
+    scope_exit close_stream([=] { ::closedir(stream); });
+
+    uintmax_t count = 0;
+    while (true) {
+      auto [str, type] = detail::posix_readdir(stream, ec);
+      static_assert(std::is_same_v<decltype(str), std::string_view>);
+      if (str == "." || str == "..") {
+        continue;
+      } else if (ec || str.empty()) {
+        break; // we're done iterating through the directory
+      } else {
+        count += remove_all_impl(fd, str, ec);
+      }
+    }
+
+    // Then, remove the now-empty directory itself.
+    if (::unlinkat(parent_directory, p.c_str(), AT_REMOVEDIR) == -1) {
+      ec = detail::capture_errno();
+      return count;
+    }
+
+    return count + 1; // the contents of the directory + the directory itself
+  }
+
+  ec = detail::capture_errno();
+
+  // If we failed to open `p` because it didn't exist, it's not an
+  // error -- it might have moved or have been deleted already.
+  if (ec == errc::no_such_file_or_directory) {
+    ec.clear();
+    return 0;
+  }
+
+  // If opening `p` failed because it wasn't a directory, remove it as
+  // a normal file instead. Note that `openat()` can return either ENOTDIR
+  // or ELOOP depending on the exact reason of the failure.
+  if (ec == errc::not_a_directory || ec == errc::too_many_symbolic_link_levels) {
+    ec.clear();
+    if (::unlinkat(parent_directory, p.c_str(), /* flags = */0) == -1) {
+      ec = detail::capture_errno();
+      return 0;
+    }
+    return 1;
+  }
+
+  // Otherwise, it's a real error -- we don't remove anything.
+  return 0;
+}
+
+} // end namespace
+
+uintmax_t __remove_all(const path& p, error_code* ec) {
+  ErrorHandler<uintmax_t> err("remove_all", ec, &p);
+  error_code mec;
+  uintmax_t count = remove_all_impl(AT_FDCWD, p, mec);
+  if (mec)
+    return err.report(mec);
+  return count;
+}
+
+#endif // REMOVE_ALL_USE_DIRECTORY_ITERATOR
+
 void __rename(const path& from, const path& to, error_code* ec) {
   ErrorHandler<void> err("rename", ec, &from, &to);
   if (detail::rename(from.c_str(), to.c_str()) == -1)

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/toctou.pass.cpp

@@ -0,0 +1,89 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-has-no-localization
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <filesystem>
+
+// Test for a time-of-check to time-of-use issue with std::filesystem::remove_all.
+//
+// Scenario:
+// The attacker wants to get directory contents deleted, to which he does not have access.
+// He has a way to get a privileged binary call `std::filesystem::remove_all()` on a
+// directory he controls, e.g. in his home directory.
+//
+// The POC sets up the `attack_dest/attack_file` which the attacker wants to have deleted.
+// The attacker repeatedly creates a directory and replaces it with a symlink from
+// `victim_del` to `attack_dest` while the victim code calls `std::filesystem::remove_all()`
+// on `victim_del`. After a few seconds the attack has succeeded and
+// `attack_dest/attack_file` is deleted.
+//
+// This is taken from https://github.com/rust-lang/wg-security-response/blob/master/patches/CVE-2022-21658/0002-Fix-CVE-2022-21658-for-UNIX-like.patch
+
+// This test requires a dylib containing the fix shipped in https://reviews.llvm.org/D118134.
+// We use UNSUPPORTED instead of XFAIL because the test might not fail reliably.
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14|15}}
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx11
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx12.{{0|1|2}}
+
+// Windows doesn't support the necessary APIs to mitigate this issue.
+// UNSUPPORTED: target={{.+}}-windows-{{.+}}
+
+#include <cstdio>
+#include <filesystem>
+#include <system_error>
+#include <thread>
+
+#include "filesystem_include.h"
+#include "filesystem_test_helper.h"
+
+int main() {
+  scoped_test_env env;
+  fs::path const tmpdir = env.create_dir("mydir");
+  fs::path const victim_del_path = tmpdir / "victim_del";
+  fs::path const attack_dest_dir = env.create_dir(tmpdir / "attack_dest");
+  fs::path const attack_dest_file = env.create_file(attack_dest_dir / "attack_file", 42);
+
+  // victim just continuously removes `victim_del`
+  bool stop = false;
+  std::thread t{[&]() {
+    while (!stop) {
+        std::error_code ec;
+        fs::remove_all(victim_del_path, ec); // ignore any error
+    }
+  }};
+
+  // attacker (could of course be in a separate process)
+  auto start_time = std::chrono::system_clock::now();
+  auto elapsed_since = [](std::chrono::system_clock::time_point const& time_point) {
+      return std::chrono::duration_cast<std::chrono::seconds>(std::chrono::system_clock::now() - time_point);
+  };
+  bool attack_succeeded = false;
+  while (elapsed_since(start_time) < std::chrono::seconds(5)) {
+    if (!fs::exists(attack_dest_file)) {
+      std::printf("Victim deleted symlinked file outside of victim_del. Attack succeeded in %lld seconds.\n",
+                  elapsed_since(start_time).count());
+      attack_succeeded = true;
+      break;
+    }
+    std::error_code ec;
+    fs::create_directory(victim_del_path, ec);
+    if (ec) {
+      continue;
+    }
+
+    fs::remove(victim_del_path);
+    fs::create_directory_symlink(attack_dest_dir, victim_del_path);
+  }
+  stop = true;
+  t.join();
+
+  return attack_succeeded ? 1 : 0;
+}