From 5c34c24520019085fdc26db126be4d451b2b2428 Mon Sep 17 00:00:00 2001 From: George Pollard <gpollard@microsoft.com> Date: Thu, 26 Oct 2023 00:46:14 +0000 Subject: [PATCH 1/2] Create temp working directory for each libfuzzer invocation --- src/agent/Cargo.lock | 1 + src/agent/coverage/fuzz/Cargo.toml | 3 +- src/agent/onefuzz-agent/Cargo.toml | 1 + src/agent/onefuzz-agent/src/validations.rs | 3 +- .../src/tasks/fuzz/libfuzzer/common.rs | 18 ++++++++---- src/agent/onefuzz/src/libfuzzer.rs | 28 +++++++++++++++++-- 6 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 5393e9b767..bf72c8d82f 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -2216,6 +2216,7 @@ dependencies = [ "serde", "serde_json", "storage-queue", + "tempfile", "tokio", "url", "uuid", diff --git a/src/agent/coverage/fuzz/Cargo.toml b/src/agent/coverage/fuzz/Cargo.toml index eef71b5fad..b6ceef10a8 100644 --- a/src/agent/coverage/fuzz/Cargo.toml +++ b/src/agent/coverage/fuzz/Cargo.toml @@ -9,7 +9,7 @@ cargo-fuzz = true [dependencies] libfuzzer-sys = "0.4" -tempfile = "3.7" +tempfile = "3.8" debuggable-module = { path = "../../debuggable-module" } @@ -34,4 +34,3 @@ name = "fuzz_target_allowlist_parse" path = "fuzz_targets/fuzz_target_allowlist_parse.rs" test = false doc = false - diff --git a/src/agent/onefuzz-agent/Cargo.toml b/src/agent/onefuzz-agent/Cargo.toml index 3e7f00a8b0..200b74a24b 100644 --- a/src/agent/onefuzz-agent/Cargo.toml +++ b/src/agent/onefuzz-agent/Cargo.toml @@ -40,6 +40,7 @@ azure_storage = { version = "0.15", default-features = false, features = [ azure_storage_blobs = { version = "0.15", default-features = false, features = [ "enable_reqwest", ] } +tempfile = "3.8" [target.'cfg(target_family = "unix")'.dependencies] diff --git a/src/agent/onefuzz-agent/src/validations.rs b/src/agent/onefuzz-agent/src/validations.rs index 93334eebb5..2908ecde03 100644 --- a/src/agent/onefuzz-agent/src/validations.rs +++ b/src/agent/onefuzz-agent/src/validations.rs @@ -101,6 +101,7 @@ async fn run_setup(setup_folder: impl AsRef<Path>) -> Result<()> { } async fn get_logs(config: ValidationConfig) -> Result<()> { + let tmp_working_dir = tempfile::tempdir()?; let setup_folder = config .setup_folder .clone() @@ -121,7 +122,7 @@ async fn get_logs(config: ValidationConfig) -> Result<()> { }, ); - let cmd = libfuzzer.build_std_command(None, None, None, None, None)?; + let cmd = libfuzzer.build_std_command(tmp_working_dir.path(), None, None, None, None, None)?; print_logs(cmd)?; Ok(()) } diff --git a/src/agent/onefuzz-task/src/tasks/fuzz/libfuzzer/common.rs b/src/agent/onefuzz-task/src/tasks/fuzz/libfuzzer/common.rs index 32f3372958..4d3624d209 100644 --- a/src/agent/onefuzz-task/src/tasks/fuzz/libfuzzer/common.rs +++ b/src/agent/onefuzz-task/src/tasks/fuzz/libfuzzer/common.rs @@ -257,6 +257,7 @@ where worker_id: usize, stats_sender: Option<&StatsSender>, ) -> Result<()> { + let tmp_working_dir = self.create_local_temp_dir().await?; let crash_dir = self.create_local_temp_dir().await?; let run_id = Uuid::new_v4(); @@ -272,7 +273,12 @@ where info!("config is: {:?}", self.config); let fuzzer = L::from_config(&self.config).await?; - let mut running = fuzzer.fuzz(crash_dir.path(), local_inputs, &inputs)?; + let mut running = fuzzer.fuzz( + tmp_working_dir.path(), + crash_dir.path(), + local_inputs, + &inputs, + )?; info!("child is: {:?}", running); @@ -376,22 +382,22 @@ where // note that collecting the dumps must be enabled by the template #[cfg(target_os = "linux")] if let Some(pid) = pid { - // expect crash dump to exist in CWD - let filename = format!("core.{pid}"); + // expect crash dump to exist in fuzzer (temp) working dir + let filename = tmp_working_dir.path().join(format!("core.{pid}")); let dest_filename = dump_file_name.as_deref().unwrap_or(OsStr::new(&filename)); let dest_path = crashdumps.local_path.join(dest_filename); match tokio::fs::rename(&filename, &dest_path).await { Ok(()) => { info!( "moved crash dump {} to output directory: {}", - filename, + filename.display(), dest_path.display() ); } Err(e) => { if e.kind() == std::io::ErrorKind::NotFound { // okay, no crash dump found - info!("no crash dump found with name: {}", filename); + info!("no crash dump found with name: {}", filename.display()); } else { return Err(e).context("moving crash dump to output directory"); } @@ -406,7 +412,7 @@ where { let dumpfile_extension = Some(std::ffi::OsStr::new("dmp")); - let mut working_dir = tokio::fs::read_dir(".").await?; + let mut working_dir = tokio::fs::read_dir(tmp_working_dir.path()).await?; let mut found_dump = false; while let Some(next) = working_dir.next_entry().await? { if next.path().extension() == dumpfile_extension { diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs index 87428b102f..c997f8e0c9 100644 --- a/src/agent/onefuzz/src/libfuzzer.rs +++ b/src/agent/onefuzz/src/libfuzzer.rs @@ -70,6 +70,7 @@ impl LibFuzzer { // Build an async `Command`. fn build_command( &self, + working_dir: &Path, fault_dir: Option<&Path>, corpus_dir: Option<&Path>, extra_corpus_dirs: Option<&[&Path]>, @@ -77,6 +78,7 @@ impl LibFuzzer { custom_arg_filter: Option<&dyn Fn(String) -> Option<String>>, ) -> Result<Command> { let std_cmd = self.build_std_command( + working_dir, fault_dir, corpus_dir, extra_corpus_dirs, @@ -96,6 +98,7 @@ impl LibFuzzer { // Build a non-async `Command`. pub fn build_std_command( &self, + working_dir: &Path, fault_dir: Option<&Path>, corpus_dir: Option<&Path>, extra_corpus_dirs: Option<&[&Path]>, @@ -103,6 +106,12 @@ impl LibFuzzer { custom_arg_filter: Option<&dyn Fn(String) -> Option<String>>, ) -> Result<std::process::Command> { let mut cmd = std::process::Command::new(&self.exe); + + // subprocess is isolated in its own working directory + // this is to prevent collisions between multiple libfuzzers + // (for example, crash dumps are generated into working directory on Windows) + cmd.current_dir(working_dir); + cmd.env(PATH, get_path_with_directory(PATH, &self.setup_dir)?) .env_remove("RUST_LOG") .stdin(Stdio::null()) @@ -250,7 +259,9 @@ impl LibFuzzer { // Verify that the libfuzzer exits with a zero return code with a known // good input, which libfuzzer works as we expect. async fn check_input(&self, input: &Path) -> Result<()> { + let tmp_working_dir = tempdir()?; let mut cmd = self.build_command( + tmp_working_dir.path(), None, None, None, @@ -294,7 +305,15 @@ impl LibFuzzer { /// least able to satisfy the fuzzer's shared library dependencies. User-authored /// dynamic loading may still fail later on, e.g. in `LLVMFuzzerInitialize()`. async fn check_help(&self) -> Result<()> { - let mut cmd = self.build_command(None, None, None, Some(&["-help=1".as_ref()]), None)?; + let tmp_working_dir = tempdir()?; + let mut cmd = self.build_command( + tmp_working_dir.path(), + None, + None, + None, + Some(&["-help=1".as_ref()]), + None, + )?; let result = cmd .spawn() @@ -326,7 +345,8 @@ impl LibFuzzer { } async fn find_missing_libraries(&self) -> Result<(Vec<String>, Vec<String>)> { - let cmd = self.build_std_command(None, None, None, None, None)?; + let tmp_working_dir = tempdir()?; + let cmd = self.build_std_command(tmp_working_dir.path(), None, None, None, None, None)?; #[cfg(target_os = "linux")] let blocking = move || dynamic_library::linux::find_missing(cmd); @@ -343,6 +363,7 @@ impl LibFuzzer { pub fn fuzz( &self, + working_dir: &Path, fault_dir: impl AsRef<Path>, corpus_dir: impl AsRef<Path>, extra_corpus_dirs: &[impl AsRef<Path>], @@ -357,6 +378,7 @@ impl LibFuzzer { let artifact_prefix = artifact_prefix(fault_dir.as_ref()); let mut cmd = self.build_command( + working_dir, Some(fault_dir.as_ref()), Some(corpus_dir.as_ref()), Some(&extra_corpus_dirs), @@ -406,8 +428,10 @@ impl LibFuzzer { corpus_dir: impl AsRef<Path>, extra_corpus_dirs: &[impl AsRef<Path>], ) -> Result<LibFuzzerMergeOutput> { + let tmp_working_dir = tempdir()?; let extra_corpus_dirs: Vec<&Path> = extra_corpus_dirs.iter().map(|x| x.as_ref()).collect(); let mut cmd = self.build_command( + tmp_working_dir.path(), None, Some(corpus_dir.as_ref()), Some(&extra_corpus_dirs), From fe8d09587620b2a7ea1db08436c8b05de0a242b4 Mon Sep 17 00:00:00 2001 From: George Pollard <gpollard@microsoft.com> Date: Mon, 30 Oct 2023 22:44:25 +0000 Subject: [PATCH 2/2] Input path must be absolute --- src/agent/onefuzz/src/libfuzzer.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs index c997f8e0c9..6949e90d8c 100644 --- a/src/agent/onefuzz/src/libfuzzer.rs +++ b/src/agent/onefuzz/src/libfuzzer.rs @@ -259,6 +259,7 @@ impl LibFuzzer { // Verify that the libfuzzer exits with a zero return code with a known // good input, which libfuzzer works as we expect. async fn check_input(&self, input: &Path) -> Result<()> { + let absolute_input = dunce::canonicalize(input)?; let tmp_working_dir = tempdir()?; let mut cmd = self.build_command( tmp_working_dir.path(), @@ -267,7 +268,7 @@ impl LibFuzzer { None, // Custom args for this run: supply the required input. In this mode, // LibFuzzer will only execute one run of fuzzing unless overridden - Some(&[input.as_ref()]), + Some(&[absolute_input.as_ref()]), // Filter out any argument starting with `-runs=` from the custom // target options, if supplied, so that it doesn't make more than // one run happen: