Cleanup forkserver exec builder (#1094)

* Don't use magic string but string constant

* Don't allow to specify multiple input files with different names

* Ensure that the file name for the current test case is unique for every fuzzer currently running

* Add note advising users to choose distinct names for the input file

* Move builder functions to more generic implementation to allow parse_afl_cmdline rewrite

* Rewrite parse_afl_cmdline to reduce code duplication

* Add remark to documentation regarding the program path

* Change behavior to allow the usage of actual AFL command lines, hopefully without breaking existing code

* Rustfmt

* Move generation of unique filename to fs

* Ensure default input filename for command executor is unique per fuzzing process

* Pass the input to the target via stdin, when no input file is specified

Previous solution of passing it via a standard file is useless, as the target does not know to read said file

* Rustfmt

---------

Co-authored-by: Dominik Maier <domenukk@gmail.com>
Co-authored-by: Dongjia "toka" Zhang <tokazerkje@outlook.com>

libafl/src/bolts/fs.rs

@@ -10,6 +10,7 @@ use std::{
     fs::{self, remove_file, File, OpenOptions},
     io::{Seek, Write},
     path::{Path, PathBuf},
+    string::String,
 };
 
 use crate::Error;
@@ -17,6 +18,13 @@ use crate::Error;
 /// The default filename to use to deliver testcases to the target
 pub const INPUTFILE_STD: &str = ".cur_input";
 
+#[must_use]
+/// Derives a filename from [`INPUTFILE_STD`] that may be used to deliver testcases to the target.
+/// It ensures the filename is unique to the fuzzer process.
+pub fn get_unique_std_input_file() -> String {
+    format!("{}_{}", INPUTFILE_STD, std::process::id())
+}
+
 /// Creates a `.{file_name}.tmp` file, and writes all bytes to it.
 /// After all bytes have been written, the tmp-file is moved to it's original `path`.
 /// This way, on the majority of operating systems, the final file will never be incomplete or racey.

libafl/src/executors/command.rs

@@ -22,7 +22,7 @@ use super::HasObservers;
 use crate::executors::{Executor, ExitKind};
 use crate::{
     bolts::{
-        fs::{InputFile, INPUTFILE_STD},
+        fs::{get_unique_std_input_file, InputFile},
         tuples::MatchName,
         AsSlice,
     },
@@ -456,7 +456,7 @@ impl CommandExecutorBuilder {
     /// Uses a default filename.
     /// Use [`Self::arg_input_file`] to specify a custom filename.
     pub fn arg_input_file_std(&mut self) -> &mut Self {
-        self.arg_input_file(INPUTFILE_STD);
+        self.arg_input_file(get_unique_std_input_file());
         self
     }
 

libafl/src/executors/forkserver.rs

@@ -26,7 +26,7 @@ use nix::{
 
 use crate::{
     bolts::{
-        fs::{InputFile, INPUTFILE_STD},
+        fs::{get_unique_std_input_file, InputFile},
         os::{dup2, pipes::Pipe},
         shmem::{ShMem, ShMemProvider, UnixShMemProvider},
         tuples::Prepend,
@@ -607,6 +607,10 @@ pub struct ForkserverExecutorBuilder<'a, SP> {
 
 impl<'a, SP> ForkserverExecutorBuilder<'a, SP> {
     /// Builds `ForkserverExecutor`.
+    /// This Forkserver will attempt to provide inputs over shared mem when `shmem_provider` is given.
+    /// Else this forkserver will pass the input to the target via `stdin`
+    /// in case no input file is specified.
+    /// If `debug_child` is set, the child will print to `stdout`/`stderr`.
     #[allow(clippy::pedantic)]
     pub fn build<OT, S>(&mut self, observers: OT) -> Result<ForkserverExecutor<OT, S, SP>, Error>
     where
@@ -691,7 +695,10 @@ impl<'a, SP> ForkserverExecutorBuilder<'a, SP> {
     {
         let input_filename = match &self.input_filename {
             Some(name) => name.clone(),
-            None => OsString::from(".cur_input"),
+            None => {
+                self.use_stdin = true;
+                OsString::from(get_unique_std_input_file())
+            }
         };
 
         let input_file = InputFile::create(input_filename)?;
@@ -821,59 +828,46 @@ impl<'a, SP> ForkserverExecutorBuilder<'a, SP> {
 
     #[must_use]
     /// Parse afl style command line
-    pub fn parse_afl_cmdline<IT, O>(mut self, args: IT) -> Self
+    ///
+    /// Replaces `@@` with the path to the input file generated by the fuzzer. If `@@` is omitted,
+    /// `stdin` is used to pass the test case instead.
+    ///
+    /// Interprets the first argument as the path to the program as long as it is not set yet.
+    /// You have to omit the program path in case you have set it already. Otherwise
+    /// it will be interpreted as a regular argument, leading to probably unintended results.
+    pub fn parse_afl_cmdline<IT, O>(self, args: IT) -> Self
     where
         IT: IntoIterator<Item = O>,
         O: AsRef<OsStr>,
     {
-        let mut res = vec![];
+        let mut moved = self;
-        let mut use_stdin = true;
+
+        let mut use_arg_0_as_program = false;
+        if moved.program.is_none() {
+            use_arg_0_as_program = true;
+        }
 
         for item in args {
-            if item.as_ref() == "@@" && use_stdin {
+            if use_arg_0_as_program {
-                use_stdin = false;
+                moved = moved.program(item);
-                res.push(OsString::from(".cur_input"));
+                // After the program has been set, unset `use_arg_0_as_program` to treat all
-            } else if let Some(name) = &self.input_filename {
+                // subsequent arguments as regular arguments
-                if name == item.as_ref() && use_stdin {
+                use_arg_0_as_program = false;
-                    use_stdin = false;
+            } else if item.as_ref() == "@@" {
-                    res.push(name.clone());
+                if let Some(name) = &moved.input_filename.clone() {
+                    // If the input file name has been modified, use this one
+                    moved = moved.arg_input_file(name);
                 } else {
-                    res.push(item.as_ref().to_os_string());
+                    moved = moved.arg_input_file_std();
                 }
             } else {
-                res.push(item.as_ref().to_os_string());
+                moved = moved.arg(item);
             }
         }
 
-        self.arguments = res;
+        // If we have not set an input file, use stdin as it is AFLs default
-        self.use_stdin = use_stdin;
+        moved.use_stdin = moved.input_filename.is_none();
-        self
+        moved
-    }
-}
-
-impl<'a> ForkserverExecutorBuilder<'a, UnixShMemProvider> {
-    /// Creates a new `AFL`-style [`ForkserverExecutor`] with the given target, arguments and observers.
-    /// This is the builder for `ForkserverExecutor`
-    /// This Forkserver will attempt to provide inputs over shared mem when `shmem_provider` is given.
-    /// Else this forkserver will try to write the input to `.cur_input` file.
-    /// If `debug_child` is set, the child will print to `stdout`/`stderr`.
-    #[must_use]
-    pub fn new() -> ForkserverExecutorBuilder<'a, UnixShMemProvider> {
-        ForkserverExecutorBuilder {
-            program: None,
-            arguments: vec![],
-            envs: vec![],
-            debug_child: false,
-            use_stdin: false,
-            uses_shmem_testcase: false,
-            is_persistent: false,
-            is_deferred_frksrv: false,
-            autotokens: None,
-            input_filename: None,
-            shmem_provider: None,
-            map_size: None,
-            real_map_size: 0,
-        }
     }
 
     /// The harness
@@ -941,16 +935,29 @@ impl<'a> ForkserverExecutorBuilder<'a, UnixShMemProvider> {
 
     #[must_use]
     /// Place the input at this position and set the filename for the input.
+    ///
+    /// Note: If you use this, you should ensure that there is only one instance using this
+    /// file at any given time.
     pub fn arg_input_file<P: AsRef<Path>>(self, path: P) -> Self {
         let mut moved = self.arg(path.as_ref());
-        moved.input_filename = Some(path.as_ref().as_os_str().to_os_string());
+
+        let path_as_string = path.as_ref().as_os_str().to_os_string();
+
+        assert!(
+            // It's only save to set the input_filename, if it does not overwrite an existing one.
+            (moved.input_filename.is_none() || moved.input_filename.unwrap() == path_as_string),
+            "Already specified an input file under a different name. This is not supported"
+        );
+
+        moved.input_filename = Some(path_as_string);
         moved
     }
 
     #[must_use]
     /// Place the input at this position and set the default filename for the input.
+    /// The filename includes the PID of the fuzzer to ensure that no two fuzzers write to the same file
     pub fn arg_input_file_std(self) -> Self {
-        self.arg_input_file(INPUTFILE_STD)
+        self.arg_input_file(get_unique_std_input_file())
     }
 
     #[must_use]
@@ -980,6 +987,33 @@ impl<'a> ForkserverExecutorBuilder<'a, UnixShMemProvider> {
         self.map_size = Some(size);
         self
     }
+}
+
+impl<'a> ForkserverExecutorBuilder<'a, UnixShMemProvider> {
+    /// Creates a new `AFL`-style [`ForkserverExecutor`] with the given target, arguments and observers.
+    /// This is the builder for `ForkserverExecutor`
+    /// This Forkserver will attempt to provide inputs over shared mem when `shmem_provider` is given.
+    /// Else this forkserver will pass the input to the target via `stdin`
+    /// in case no input file is specified.
+    /// If `debug_child` is set, the child will print to `stdout`/`stderr`.
+    #[must_use]
+    pub fn new() -> ForkserverExecutorBuilder<'a, UnixShMemProvider> {
+        ForkserverExecutorBuilder {
+            program: None,
+            arguments: vec![],
+            envs: vec![],
+            debug_child: false,
+            use_stdin: false,
+            uses_shmem_testcase: false,
+            is_persistent: false,
+            is_deferred_frksrv: false,
+            autotokens: None,
+            input_filename: None,
+            shmem_provider: None,
+            map_size: None,
+            real_map_size: 0,
+        }
+    }
 
     /// Shmem provider for forkserver's shared memory testcase feature.
     pub fn shmem_provider<SP: ShMemProvider>(