From 85d45d26e1e978301dd537e275c710a58044f0d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Kj=C3=A4ll?= Date: Sat, 4 Mar 2023 22:49:41 +0100 Subject: [PATCH] remove_dir_all have TOCTOU race condition (#1628) * remove_dir_all have TOCTOU race condition reported in GHSA-mc8h-8q98-g5hr * Replace GHSA description with an excerpt form upstream changelog, add GHSA to references --------- Co-authored-by: Sergey "Shnatsel" Davidoff --- crates/remove_dir_all/RUSTSEC-0000-0000.md | 67 ++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 crates/remove_dir_all/RUSTSEC-0000-0000.md diff --git a/crates/remove_dir_all/RUSTSEC-0000-0000.md b/crates/remove_dir_all/RUSTSEC-0000-0000.md new file mode 100644 index 00000000..9681cca0 --- /dev/null +++ b/crates/remove_dir_all/RUSTSEC-0000-0000.md @@ -0,0 +1,67 @@ +```toml +[advisory] +id = "RUSTSEC-0000-0000" +package = "remove_dir_all" +date = "2023-02-24" +url = "https://github.com/XAMPPRocky/remove_dir_all/commit/7247a8b6ee59fc99bbb69ca6b3ca4bfd8c809ead" +references = ["https://github.com/advisories/GHSA-mc8h-8q98-g5hr"] +keywords = ["TOCTOU"] +aliases = ["GHSA-mc8h-8q98-g5hr"] + +[affected] +functions = { "remove_dir_all::remove_dir_all" = ["< 0.8.0"], "remove_dir_all::remove_dir_contents" = ["< 0.8.0"], "remove_dir_all::ensure_empty_dir" = ["< 0.8.0"] } + +[versions] +patched = [">= 0.8.0"] +``` + +# Race Condition Enabling Link Following and Time-of-check Time-of-use (TOCTOU) + +The remove_dir_all crate is a Rust library that offers additional features over the Rust +standard library fs::remove_dir_all function. + +It was possible to trick a privileged process doing a recursive delete in an +attacker controlled directory into deleting privileged files, on all operating systems. + +For instance, consider deleting a tree called 'etc' in a parent directory +called 'p'. Between calling `remove_dir_all("a")` and remove_dir_all("a") +actually starting its work, the attacker can move 'p' to 'p-prime', and +replace 'p' with a symlink to '/'. Then the privileged process deletes 'p/etc' +which is actually /etc, and now your system is broken. There are some +mitigations for this exact scenario, such as CWD relative file lookup, but +they are not guaranteed - any code using absolute paths will not have that +protection in place. + +The same attack could be performed at any point in the directory tree being +deleted: if 'a' contains a child directory called 'etc', attacking the +deletion by replacing 'a' with a link is possible. + +The new code in this release mitigates the attack within the directory tree +being deleted by using file-handle relative operations: to open 'a/etc', the +path 'etc' relative to 'a' is opened, where 'a' is represented by a file +descriptor (Unix) or handle (Windows). With the exception of the entry points +into the directory deletion logic, this is robust against manipulation of the +directory hierarchy, and remove_dir_all will only delete files and directories +contained in the tree it is deleting. + +The entry path however is a challenge - as described above, there are some +potential mitigations, but since using them must be done by the calling code, +it is hard to be confident about the security properties of the path based +interface. + +The new extension trait `RemoveDir` provides an interface where it is much +harder to get it wrong. + +`somedir.remove_dir_contents("name-of-child")`. + +Callers can then make their own security evaluation about how to securely get +a directory handle. That is still not particularly obvious, and we're going to +follow up with a helper of some sort (probably in the `fs_at` crate). Once +that is available, the path based entry points will get deprecated. + +In the interim, processes that might run with elevated privileges should +figure out how to securely identify the directory they are going to delete, to +avoid the initial race. Pragmatically, other processes should be fine with the +path based entry points : this is the same interface `std::fs::remove_dir_all` +offers, and an unprivileged process running in an attacker controlled +directory can't do anything that the attacker can't already do.