[libcxx-commits] [PATCH] D118134: [libc++] Fix TOCTOU issue with std::filesystem::remove_all

Fri Jan 28 03:00:57 PST 2022

mstorsjo added a comment.

Thanks, noted. I don't believe I have the bandwidth to fix this right now before the 14.x branch early next week though, so the current form of the patch seems sensible wrt Windows I think.

In D118134#3277706 <https://reviews.llvm.org/D118134#3277706>, @mati865 wrote:

> Looking at Rust's fix over https://github.com/rust-lang/rust/commit/4f0ad1c92ca08da6e8dc17838070975762f59714 seems like there is API added in Windows 10 to solve it (I don't know how effective though).

The use of `SetFileInformationByHandle(FileDispositionInfoEx)` with `FILE_DISPOSITION_FLAG_POSIX_SEMANTICS` doesn't seem to be the core of the fix. The core of the fix is to rewrite directory iteration with something that operates on an open handle, instead of something given a path. In the Rust fix, this is done with `GetFileInformationByHandleEx(FileIdBothDirectoryInfo)` (which I would believe exists earlier).

So we could open a handle to the intended path with `FILE_FLAG_OPEN_REPARSE_POINT` (so it doesn't follow symlinks), then inspect whether it's a regular file or a directory, and if a directory, iterate over its contents without closing the handle and using a path name again. (Currently we use the generic directory iterators, which are built on top of `FindFirstFileW`/`FindNextFileW`.)

================
Comment at: libcxx/src/filesystem/operations.cpp:49

+#include <dirent.h>
+
----------------
This header can't be included unconditionally on all OSes.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118134/new/

https://reviews.llvm.org/D118134