Skip to content

Commit 92c92fb

Browse files
committed
Auto merge of #93111 - pietroalbini:pa-cve-2022-21658-beta, r=Mark-Simulacrum
[beta] Fix CVE-2022-21658 See https://blog.rust-lang.org/2022/01/20/cve-2022-21658.html. Patches reviewed by `@m-ou-se.` Also backports: * resolve rustfmt issue with generated files #92912 * rustdoc: fix intra-link for generic trait impls #92792 * Fix rust logo style #92764 r? `@ghost`
2 parents 98a701b + 046f8b8 commit 92c92fb

File tree

18 files changed

+998
-63
lines changed

18 files changed

+998
-63
lines changed

library/std/src/fs.rs

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

library/std/src/fs/tests.rs

+70
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ use crate::fs::{self, File, OpenOptions};
44
use crate::io::{ErrorKind, SeekFrom};
55
use crate::path::Path;
66
use crate::str;
7+
use crate::sync::Arc;
78
use crate::sys_common::io::test::{tmpdir, TempDir};
89
use crate::thread;
10+
use crate::time::{Duration, Instant};
911

1012
use rand::{rngs::StdRng, RngCore, SeedableRng};
1113

@@ -601,6 +603,21 @@ fn recursive_rmdir_of_symlink() {
601603
assert!(canary.exists());
602604
}
603605

606+
#[test]
607+
fn recursive_rmdir_of_file_fails() {
608+
// test we do not delete a directly specified file.
609+
let tmpdir = tmpdir();
610+
let canary = tmpdir.join("do_not_delete");
611+
check!(check!(File::create(&canary)).write(b"foo"));
612+
let result = fs::remove_dir_all(&canary);
613+
#[cfg(unix)]
614+
error!(result, "Not a directory");
615+
#[cfg(windows)]
616+
error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid.
617+
assert!(result.is_err());
618+
assert!(canary.exists());
619+
}
620+
604621
#[test]
605622
// only Windows makes a distinction between file and directory symlinks.
606623
#[cfg(windows)]
@@ -620,6 +637,59 @@ fn recursive_rmdir_of_file_symlink() {
620637
}
621638
}
622639

640+
#[test]
641+
#[ignore] // takes too much time
642+
fn recursive_rmdir_toctou() {
643+
// Test for time-of-check to time-of-use issues.
644+
//
645+
// Scenario:
646+
// The attacker wants to get directory contents deleted, to which he does not have access.
647+
// He has a way to get a privileged Rust binary call `std::fs::remove_dir_all()` on a
648+
// directory he controls, e.g. in his home directory.
649+
//
650+
// The POC sets up the `attack_dest/attack_file` which the attacker wants to have deleted.
651+
// The attacker repeatedly creates a directory and replaces it with a symlink from
652+
// `victim_del` to `attack_dest` while the victim code calls `std::fs::remove_dir_all()`
653+
// on `victim_del`. After a few seconds the attack has succeeded and
654+
// `attack_dest/attack_file` is deleted.
655+
let tmpdir = tmpdir();
656+
let victim_del_path = tmpdir.join("victim_del");
657+
let victim_del_path_clone = victim_del_path.clone();
658+
659+
// setup dest
660+
let attack_dest_dir = tmpdir.join("attack_dest");
661+
let attack_dest_dir = attack_dest_dir.as_path();
662+
fs::create_dir(attack_dest_dir).unwrap();
663+
let attack_dest_file = tmpdir.join("attack_dest/attack_file");
664+
File::create(&attack_dest_file).unwrap();
665+
666+
let drop_canary_arc = Arc::new(());
667+
let drop_canary_weak = Arc::downgrade(&drop_canary_arc);
668+
669+
eprintln!("x: {:?}", &victim_del_path);
670+
671+
// victim just continuously removes `victim_del`
672+
thread::spawn(move || {
673+
while drop_canary_weak.upgrade().is_some() {
674+
let _ = fs::remove_dir_all(&victim_del_path_clone);
675+
}
676+
});
677+
678+
// attacker (could of course be in a separate process)
679+
let start_time = Instant::now();
680+
while Instant::now().duration_since(start_time) < Duration::from_secs(1000) {
681+
if !attack_dest_file.exists() {
682+
panic!(
683+
"Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.",
684+
Instant::now().duration_since(start_time)
685+
);
686+
}
687+
let _ = fs::create_dir(&victim_del_path);
688+
let _ = fs::remove_dir(&victim_del_path);
689+
let _ = symlink_dir(attack_dest_dir, &victim_del_path);
690+
}
691+
}
692+
623693
#[test]
624694
fn unicode_path_is_dir() {
625695
assert!(Path::new(".").is_dir());

0 commit comments

Comments
 (0)