Skip to content

Commit

Permalink
Auto merge of #93110 - pietroalbini:pa-cve-2022-21658-stable, r=pietr…
Browse files Browse the repository at this point in the history
…oalbini

[stable] Fix CVE 2022 21658 and prepare 1.58.1

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

r? `@ghost`
cc `@rust-lang/release` `@rust-lang/security`
  • Loading branch information
bors committed Jan 20, 2022
2 parents 02072b4 + 1c63ec4 commit 08182e4
Show file tree
Hide file tree
Showing 27 changed files with 949 additions and 95 deletions.
15 changes: 15 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
@@ -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)
==========================

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
12 changes: 8 additions & 4 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
Expand Down
70 changes: 70 additions & 0 deletions library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)]
Expand All @@ -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());
Expand Down
Loading

0 comments on commit 08182e4

Please sign in to comment.