diff --git a/libcxx/include/__filesystem/directory_options.h b/libcxx/include/__filesystem/directory_options.h --- a/libcxx/include/__filesystem/directory_options.h +++ b/libcxx/include/__filesystem/directory_options.h @@ -22,7 +22,8 @@ enum class _LIBCPP_ENUM_VIS directory_options : unsigned char { none = 0, follow_directory_symlink = 1, - skip_permission_denied = 2 + skip_permission_denied = 2, + __disallow_root_directory_symlink = 4 }; _LIBCPP_INLINE_VISIBILITY diff --git a/libcxx/src/filesystem/directory_iterator.cpp b/libcxx/src/filesystem/directory_iterator.cpp --- a/libcxx/src/filesystem/directory_iterator.cpp +++ b/libcxx/src/filesystem/directory_iterator.cpp @@ -195,7 +195,14 @@ __dir_stream(const path& root, directory_options opts, error_code& ec) : __stream_(nullptr), __root_(root) { - if ((__stream_ = ::opendir(root.c_str())) == nullptr) { + if (bool(opts & directory_options::__disallow_root_directory_symlink)) { + int fd = ::openat(AT_FDCWD, root.c_str(), O_CLOEXEC | O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + __stream_ = ::fdopendir(fd); + } else { + __stream_ = ::opendir(root.c_str()); + } + + if (__stream_ == nullptr) { ec = detail::capture_errno(); const bool allow_eacess = bool(opts & directory_options::skip_permission_denied); diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp --- a/libcxx/src/filesystem/operations.cpp +++ b/libcxx/src/filesystem/operations.cpp @@ -1340,26 +1340,47 @@ namespace { +// Removes the file at the given path and returns the number of files removed, i.e. 0 or 1. +// If the path doesn't exist, 0 is returned and it is not considered an error. +// If any other error occurs, `ec` is set accordingly and 0 is returned. +uintmax_t remove_if_exists_impl(path const& p, error_code& ec) { + if (__remove(p, &ec)) { + return 1; + } else if (ec == errc::no_such_file_or_directory) { + ec.clear(); + } + return 0; +} + +// Removes all files in the directory represented by `p` and subdirectories. +// This function doesn't follow symlinks, and it doesn't remove anything if +// `p` is a symlink. +// +// If `p` doesn't exist, 0 is returned (this is not considered an error). +// If any other error occurs, `ec` is set and `-1` is returned. +// Otherwise, the number of entries deleted is returned. uintmax_t remove_all_impl(path const& p, error_code& ec) { - const auto npos = static_cast(-1); - const file_status st = __symlink_status(p, &ec); - if (ec) - return npos; - uintmax_t count = 1; - if (is_directory(st)) { - for (directory_iterator it(p, ec); !ec && it != directory_iterator(); - it.increment(ec)) { - auto other_count = remove_all_impl(it->path(), ec); + uintmax_t count = 0; + + // This `is_directory` check is just an optimization to avoid trying to create + // a `directory_iterator` on something that is definitely not a directory. + // `directory_iterator` won't allow being constructed on something that is not + // a directory anyway (including if a symlink is sneaked in). + if (is_directory(p)) { + directory_iterator it(p, directory_options::__disallow_root_directory_symlink, ec), end; + if (ec && ec == errc::no_such_file_or_directory) { + ec.clear(); + return count; + } + + for (; !ec && it != end; it.increment(ec)) { + count += remove_all_impl(it->path(), ec); if (ec) - return npos; - count += other_count; + return count; } - if (ec) - return npos; } - if (!__remove(p, &ec)) - return npos; - return count; + + return count + remove_if_exists_impl(p, ec); } } // end namespace @@ -1368,12 +1389,18 @@ ErrorHandler err("remove_all", ec, &p); error_code mec; - auto count = remove_all_impl(p, mec); - if (mec) { - if (mec == errc::no_such_file_or_directory) - return 0; + const file_status status = symlink_status(p, mec); + if (mec) + return mec == errc::no_such_file_or_directory ? 0 : err.report(mec); + + // We can't just call `remove_all_impl` immediately because that wouldn't delete + // `p` if it is a symlink. We don't have to worry about races here -- if `p` + // becomes a symlink between `symlink_status` above and `remove_all_impl` + // below, `remove_all_impl` won't remove it. + uintmax_t count = is_symlink(status) ? remove_if_exists_impl(p, mec) + : remove_all_impl(p, mec); + if (mec) return err.report(mec); - } return count; } diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/toctou.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/toctou.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/toctou.pass.cpp @@ -0,0 +1,81 @@ +//===----------------------------------------------------------------------===// +// +// 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: no-exceptions + +// + +// 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 + +#include +#include +#include +#include + +int main() { + std::filesystem::path tmpdir = "/tmp/mydir"; + std::filesystem::path victim_del_path = tmpdir / "victim_del"; + + // setup dest + std::filesystem::path attack_dest_dir = tmpdir / "attack_dest"; + std::filesystem::create_directories(attack_dest_dir); + std::filesystem::path attack_dest_file = attack_dest_dir / "attack_file"; + { std::ofstream of(attack_dest_file); } + + // victim just continuously removes `victim_del` + bool stop = false; + std::thread t{[&]() { + while (!stop) { + try { + std::filesystem::remove_all(victim_del_path); + } catch (std::filesystem::filesystem_error const&) { + // ignore + } + } + }}; + + // attacker (could of course be in a separate process) + auto start_time = std::chrono::system_clock::now(); + auto elapsed_since = [](auto const& time_point) { + return std::chrono::duration_cast(std::chrono::system_clock::now() - time_point); + }; + bool attack_succeeded = false; + while (elapsed_since(start_time) < std::chrono::seconds(5)) { + if (!std::filesystem::exists(attack_dest_file)) { + std::cout << "Victim deleted symlinked file outside of victim_del. Attack succeeded in " + << elapsed_since(start_time).count() << " seconds." << std::endl; + attack_succeeded = true; + break; + } + try { + std::filesystem::create_directory(victim_del_path); + } catch (std::filesystem::filesystem_error const&) { + continue; + } + std::filesystem::remove(victim_del_path); + std::filesystem::create_directory_symlink(attack_dest_dir, victim_del_path); + } + stop = true; + t.join(); + + return attack_succeeded ? 1 : 0; +}