Auto merge of rust-lang#93110 - pietroalbini:pa-cve-2022-21658-stable, r=pietroalbini

[stable] Fix CVE 2022 21658 and prepare 1.58.1

Followup to rust-lang#93071. Includes the fix for CVE-2022-21658.

r? `@ghost`
cc `@rust-lang/release` `@rust-lang/security`

Author: bors

RELEASES.md

@@ -1,3 +1,18 @@
+Version 1.58.1 (2022-01-19)
+===========================
+
+* Fix race condition in `std::fs::remove_dir_all` ([CVE-2022-21658])
+* [Handle captured arguments in the `useless_format` Clippy lint][clippy/8295]
+* [Move `non_send_fields_in_send_ty` Clippy lint to nursery][clippy/8075]
+* [Fix wrong error message displayed when some imports are missing][91254]
+* [Fix rustfmt not formatting generated files from stdin][92912]
+
+[CVE-2022-21658]: https://www.cve.org/CVERecord?id=CVE-2022-21658]
+[91254]: https://github.com/rust-lang/rust/pull/91254
+[92912]: https://github.com/rust-lang/rust/pull/92912
+[clippy/8075]: https://github.com/rust-lang/rust-clippy/pull/8075
+[clippy/8295]: https://github.com/rust-lang/rust-clippy/pull/8295
+
 Version 1.58.0 (2022-01-13)
 ==========================
 

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -536,7 +536,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // This helps us avoid overflow: see issue #72839
         // Since compilation is already guaranteed to fail, this is just
         // to try to show the 'nicest' possible errors to the user.
-        if obligation.references_error() {
+        // We don't check for errors in the `ParamEnv` - in practice,
+        // it seems to cause us to be overly aggressive in deciding
+        // to give up searching for candidates, leading to spurious errors.
+        if obligation.predicate.references_error() {
             return;
         }
 

library/std/src/fs.rs

@@ -2045,13 +2045,17 @@ pub fn remove_dir<P: AsRef<Path>>(path: P) -> io::Result<()> {
 ///
 /// # Platform-specific behavior
 ///
-/// This function currently corresponds to `opendir`, `lstat`, `rm` and `rmdir` functions on Unix
-/// and the `FindFirstFile`, `GetFileAttributesEx`, `DeleteFile`, and `RemoveDirectory` functions
-/// on Windows.
-/// Note that, this [may change in the future][changes].
+/// This function currently corresponds to `openat`, `fdopendir`, `unlinkat` and `lstat` functions
+/// on Unix (except for macOS before version 10.10 and REDOX) and the `CreateFileW`,
+/// `GetFileInformationByHandleEx`, `SetFileInformationByHandle`, and `NtOpenFile` functions on
+/// Windows. Note that, this [may change in the future][changes].
 ///
 /// [changes]: io#platform-specific-behavior
 ///
+/// On macOS before version 10.10 and REDOX this function is not protected against time-of-check to
+/// time-of-use (TOCTOU) race conditions, and should not be used in security-sensitive code on
+/// those platforms. All other platforms are protected.
+///
 /// # Errors
 ///
 /// See [`fs::remove_file`] and [`fs::remove_dir`].

library/std/src/fs/tests.rs

@@ -4,8 +4,10 @@ use crate::fs::{self, File, OpenOptions};
 use crate::io::{ErrorKind, SeekFrom};
 use crate::path::Path;
 use crate::str;
+use crate::sync::Arc;
 use crate::sys_common::io::test::{tmpdir, TempDir};
 use crate::thread;
+use crate::time::{Duration, Instant};
 
 use rand::{rngs::StdRng, RngCore, SeedableRng};
 
@@ -602,6 +604,21 @@ fn recursive_rmdir_of_symlink() {
     assert!(canary.exists());
 }
 
+#[test]
+fn recursive_rmdir_of_file_fails() {
+    // test we do not delete a directly specified file.
+    let tmpdir = tmpdir();
+    let canary = tmpdir.join("do_not_delete");
+    check!(check!(File::create(&canary)).write(b"foo"));
+    let result = fs::remove_dir_all(&canary);
+    #[cfg(unix)]
+    error!(result, "Not a directory");
+    #[cfg(windows)]
+    error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid.
+    assert!(result.is_err());
+    assert!(canary.exists());
+}
+
 #[test]
 // only Windows makes a distinction between file and directory symlinks.
 #[cfg(windows)]
@@ -621,6 +638,59 @@ fn recursive_rmdir_of_file_symlink() {
     }
 }
 
+#[test]
+#[ignore] // takes too much time
+fn recursive_rmdir_toctou() {
+    // Test for time-of-check to time-of-use issues.
+    //
+    // Scenario:
+    // The attacker wants to get directory contents deleted, to which he does not have access.
+    // He has a way to get a privileged Rust binary call `std::fs::remove_dir_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::fs::remove_dir_all()`
+    // on `victim_del`. After a few seconds the attack has succeeded and
+    // `attack_dest/attack_file` is deleted.
+    let tmpdir = tmpdir();
+    let victim_del_path = tmpdir.join("victim_del");
+    let victim_del_path_clone = victim_del_path.clone();
+
+    // setup dest
+    let attack_dest_dir = tmpdir.join("attack_dest");
+    let attack_dest_dir = attack_dest_dir.as_path();
+    fs::create_dir(attack_dest_dir).unwrap();
+    let attack_dest_file = tmpdir.join("attack_dest/attack_file");
+    File::create(&attack_dest_file).unwrap();
+
+    let drop_canary_arc = Arc::new(());
+    let drop_canary_weak = Arc::downgrade(&drop_canary_arc);
+
+    eprintln!("x: {:?}", &victim_del_path);
+
+    // victim just continuously removes `victim_del`
+    thread::spawn(move || {
+        while drop_canary_weak.upgrade().is_some() {
+            let _ = fs::remove_dir_all(&victim_del_path_clone);
+        }
+    });
+
+    // attacker (could of course be in a separate process)
+    let start_time = Instant::now();
+    while Instant::now().duration_since(start_time) < Duration::from_secs(1000) {
+        if !attack_dest_file.exists() {
+            panic!(
+                "Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.",
+                Instant::now().duration_since(start_time)
+            );
+        }
+        let _ = fs::create_dir(&victim_del_path);
+        let _ = fs::remove_dir(&victim_del_path);
+        let _ = symlink_dir(attack_dest_dir, &victim_del_path);
+    }
+}
+
 #[test]
 fn unicode_path_is_dir() {
     assert!(Path::new(".").is_dir());