Fix hardcoded BacktraceObserver (#530)

* refactor BacktraceObserver and InProcessForkExecutor

* cleanup

* fix improcess

* fix

* mormanti

* win fix

* clippy

* fix backtrace_baby_fuzzers/command_executor

* win fix

* clippy

fuzzers/backtrace_baby_fuzzers/c_code_with_fork_executor/src/main.rs

@@ -4,7 +4,12 @@ use libafl::bolts::shmem::ShMemProvider;
 use libafl::bolts::AsSlice;
 use libafl::observers::ConstMapObserver;
 use libafl::{
-    bolts::{current_nanos, rands::StdRand, shmem::UnixShMemProvider, tuples::tuple_list},
+    bolts::{
+        current_nanos,
+        rands::StdRand,
+        shmem::{ShMem, StdShMemProvider},
+        tuples::tuple_list,
+    },
     corpus::{InMemoryCorpus, OnDiskCorpus, QueueCorpusScheduler},
     events::SimpleEventManager,
     executors::InProcessForkExecutor,
@@ -34,7 +39,7 @@ extern "C" {
 
 #[allow(clippy::similar_names)]
 pub fn main() {
-    let shmem_provider = UnixShMemProvider::new().unwrap();
+    let mut shmem_provider = StdShMemProvider::new().unwrap();
     unsafe { create_shmem_array() };
     let map_ptr = unsafe { get_ptr() };
     let mut harness = |input: &BytesInput| {
@@ -46,8 +51,12 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = unsafe { ConstMapObserver::<u8, 3>::new_from_ptr("signals", map_ptr) };
     // Create a stacktrace observer
-    let bt_observer =
+    let mut bt = shmem_provider.new_shmem_object::<Option<u64>>().unwrap();
-        BacktraceObserver::new("BacktraceObserver", libafl::observers::HarnessType::FFI);
+    let bt_observer = BacktraceObserver::new(
+        "BacktraceObserver",
+        unsafe { bt.as_object_mut::<Option<u64>>() },
+        libafl::observers::HarnessType::Child,
+    );
 
     // The state of the edges feedback.
     let feedback_state = MapFeedbackState::with_observer(&observer);

fuzzers/backtrace_baby_fuzzers/c_code_with_inprocess_executor/src/main.rs

@@ -40,8 +40,12 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = unsafe { ConstMapObserver::<u8, 3>::new_from_ptr("signals", array_ptr) };
     // Create a stacktrace observer
-    let bt_observer =
+    let mut bt = None;
-        BacktraceObserver::new("BacktraceObserver", libafl::observers::HarnessType::FFI);
+    let bt_observer = BacktraceObserver::new(
+        "BacktraceObserver",
+        &mut bt,
+        libafl::observers::HarnessType::InProcess,
+    );
 
     // The state of the edges feedback.
     let feedback_state = MapFeedbackState::with_observer(&observer);

fuzzers/backtrace_baby_fuzzers/command_executor/Cargo.toml

@@ -13,6 +13,9 @@ codegen-units = 1
 opt-level = 3
 debug = true
 
+[build-dependencies]
+cc = "*"
+
 [dependencies]
 libafl = { path = "../../../libafl/" }
 ahash = { version = "0.7"} # another hash

fuzzers/backtrace_baby_fuzzers/command_executor/build.rs

@@ -0,0 +1,11 @@
+use std::env;
+
+fn main() {
+    let cwd = env::current_dir().unwrap().to_string_lossy().to_string();
+    let mut cmd = cc::Build::new().get_compiler().to_command();
+    cmd.args(&["src/test_command.c", "-o"])
+        .arg(&format!("{}/test_command", &cwd))
+        .arg("-fsanitize=address")
+        .status()
+        .unwrap();
+}

fuzzers/backtrace_baby_fuzzers/rust_code_with_fork_executor/src/main.rs

@@ -7,7 +7,7 @@ use libafl::{
     bolts::{
         current_nanos,
         rands::StdRand,
-        shmem::{unix_shmem, ShMemProvider},
+        shmem::{unix_shmem, ShMem, ShMemProvider},
         tuples::tuple_list,
         AsMutSlice, AsSlice,
     },
@@ -33,6 +33,7 @@ pub fn main() {
     let mut shmem_provider = unix_shmem::UnixShMemProvider::new().unwrap();
     let mut signals = shmem_provider.new_shmem(16).unwrap();
     let mut signals_clone = signals.clone();
+    let mut bt = shmem_provider.new_shmem_object::<Option<u64>>().unwrap();
 
     let mut signals_set = |idx: usize| {
         let a = signals.as_mut_slice();
@@ -68,8 +69,11 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = StdMapObserver::new("signals", signals_clone.as_mut_slice());
     // Create a stacktrace observer
-    let bt_observer =
+    let bt_observer = BacktraceObserver::new(
-        BacktraceObserver::new("BacktraceObserver", libafl::observers::HarnessType::RUST);
+        "BacktraceObserver",
+        unsafe { bt.as_object_mut::<Option<u64>>() },
+        libafl::observers::HarnessType::Child,
+    );
 
     // The state of the edges feedback.
     let feedback_state = MapFeedbackState::with_observer(&observer);

fuzzers/backtrace_baby_fuzzers/rust_code_with_inprocess_executor/src/main.rs

@@ -62,8 +62,12 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = StdMapObserver::new("signals", unsafe { &mut SIGNALS });
     // Create a stacktrace observer to add the observers tuple
-    let bt_observer =
+    let mut bt = None;
-        BacktraceObserver::new("BacktraceObserver", libafl::observers::HarnessType::RUST);
+    let bt_observer = BacktraceObserver::new(
+        "BacktraceObserver",
+        &mut bt,
+        libafl::observers::HarnessType::InProcess,
+    );
 
     // The state of the edges feedback.
     let feedback_state = MapFeedbackState::with_observer(&observer);

libafl/src/bolts/shmem.rs

@@ -172,6 +172,32 @@ pub trait ShMem: Sized + Debug + Clone + AsSlice<u8> + AsMutSlice<u8> {
         self.len() == 0
     }
 
+    /// Convert to an owned object reference
+    ///
+    /// # Safety
+    /// This function is not safe as the object may be not initialized.
+    /// The user is responsible to initialize the object with something like
+    /// `*shmem.as_object_mut::<T>() = T::new();`
+    unsafe fn as_object<T: Sized + 'static>(&self) -> &T {
+        assert!(self.len() >= core::mem::size_of::<T>());
+        (self.as_slice().as_ptr() as *const () as *const T)
+            .as_ref()
+            .unwrap()
+    }
+
+    /// Convert to an owned object mutable reference
+    ///
+    /// # Safety
+    /// This function is not safe as the object may be not initialized.
+    /// The user is responsible to initialize the object with something like
+    /// `*shmem.as_object_mut::<T>() = T::new();`
+    unsafe fn as_object_mut<T: Sized + 'static>(&mut self) -> &mut T {
+        assert!(self.len() >= core::mem::size_of::<T>());
+        (self.as_mut_slice().as_mut_ptr() as *mut () as *mut T)
+            .as_mut()
+            .unwrap()
+    }
+
     /// Get the description of the shared memory mapping
     fn description(&self) -> ShMemDescription {
         ShMemDescription {
@@ -207,6 +233,19 @@ pub trait ShMemProvider: Clone + Default + Debug {
     /// Get a mapping given its id and size
     fn shmem_from_id_and_size(&mut self, id: ShMemId, size: usize) -> Result<Self::ShMem, Error>;
 
+    /// Create a new shared memory mapping to hold an object of the given type
+    fn new_shmem_object<T: Sized + 'static>(&mut self) -> Result<Self::ShMem, Error> {
+        self.new_shmem(core::mem::size_of::<T>())
+    }
+
+    /// Get a mapping given its id to hold an object of the given type
+    fn shmem_object_from_id<T: Sized + 'static>(
+        &mut self,
+        id: ShMemId,
+    ) -> Result<Self::ShMem, Error> {
+        self.shmem_from_id_and_size(id, core::mem::size_of::<T>())
+    }
+
     /// Get a mapping given a description
     fn shmem_from_description(
         &mut self,

libafl/src/feedbacks/new_hash_feedback.rs

@@ -94,7 +94,7 @@ where
 
     fn update_hash_set(&mut self, value: T) -> Result<bool, Error> {
         let r = self.hash_set.insert(value);
-        println!("Got r={}, the hashset is {:?}", r, &self.hash_set);
+        // println!("Got r={}, the hashset is {:?}", r, &self.hash_set);
         Ok(r)
     }
 }

libafl/src/observers/stacktrace.rs

@@ -1,25 +1,19 @@
 //! the ``StacktraceObserver`` looks up the stacktrace on the execution thread and computes a hash for it for dedupe
 
 use crate::{
-    bolts::{
+    bolts::{ownedref::OwnedRefMut, tuples::Named},
-        shmem::{ShMemProvider, StdShMemProvider},
-        tuples::Named,
-        AsMutSlice, AsSlice,
-    },
     executors::ExitKind,
     inputs::Input,
     observers::Observer,
     Error,
 };
-use ahash::AHasher;
+
 use backtrace::Backtrace;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
 use std::{
-    collections::hash_map::DefaultHasher,
     fmt::Debug,
     fs::{self, File},
-    hash::Hasher,
     io::Read,
     path::Path,
     process::ChildStderr,
@@ -27,197 +21,121 @@ use std::{
 
 use super::ObserverWithHashField;
 
-type StdShMem = <StdShMemProvider as ShMemProvider>::ShMem;
-/// A struct that stores needed information to persist the backtrace across prcesses/runs
-#[derive(Debug)]
-pub enum BacktraceHashValueWrapper {
-    /// shared memory instance
-    Shmem(Box<StdShMem>),
-    /// static variable
-    StaticVariable((u64, u64)),
-    /// Neither is set
-    None,
-}
-
-impl BacktraceHashValueWrapper {
-    /// store a hash value in the [`BacktraceHashValueWrapper`]
-    fn store_stacktrace_hash(&mut self, bt_hash: u64, input_hash: u64) {
-        match self {
-            Self::Shmem(shmem) => {
-                let map = shmem.as_mut_slice();
-                let bt_hash_bytes = bt_hash.to_be_bytes();
-                let input_hash_bytes = input_hash.to_be_bytes();
-                map.copy_from_slice(&[bt_hash_bytes, input_hash_bytes].concat());
-            }
-            Self::StaticVariable(_) => {
-                *self = Self::StaticVariable((bt_hash, input_hash));
-            }
-            Self::None => panic!("BacktraceSharedMemoryWrapper is not set yet!"),
-        }
-    }
-
-    /// get the hash value from the [`BacktraceHashValueWrapper`]
-    fn get_stacktrace_hash(&self) -> Result<(u64, u64), Error> {
-        match &self {
-            Self::Shmem(shmem) => {
-                let map = shmem.as_slice();
-                Ok((
-                    u64::from_be_bytes(map[0..8].try_into()?),
-                    u64::from_be_bytes(map[8..16].try_into()?),
-                ))
-            }
-            Self::StaticVariable(hash_tuple) => Ok(*hash_tuple),
-            Self::None => Err(Error::IllegalState(
-                "BacktraceSharedMemoryWrapper is not set yet!".into(),
-            )),
-        }
-    }
-}
-
-// Used for fuzzers not running in the same process
-/// Static variable storing shared memory information
-pub static mut BACKTRACE_HASH_VALUE: BacktraceHashValueWrapper = BacktraceHashValueWrapper::None;
-
 /// Collects the backtrace via [`Backtrace`] and [`Debug`]
 /// ([`Debug`] is currently used for dev purposes, symbols hash will be used eventually)
 #[must_use]
 pub fn collect_backtrace() -> u64 {
     let b = Backtrace::new();
+    if b.frames().is_empty() {
+        return 0;
+    }
+    let mut hash = 0;
+    for frame in &b.frames()[1..] {
+        hash ^= frame.ip() as u64;
+    }
     // will use symbols later
-    let trace = format!("{:?}", b);
+    // let trace = format!("{:?}", b);
-    eprintln!("{}", trace);
+    // eprintln!("{}", trace);
-    let mut hasher = AHasher::new_with_keys(0, 0);
+    // println!(
-    hasher.write(trace.as_bytes());
+    //     "backtrace collected with hash={} at pid={}",
-    let hash = hasher.finish();
+    //     hash,
-    println!(
+    //     std::process::id()
-        "backtrace collected with hash={} at pid={}",
+    // );
-        hash,
-        std::process::id()
-    );
     hash
 }
 
 /// An enum encoding the types of harnesses
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
 pub enum HarnessType {
-    /// Harness type when the harness is rust code
+    /// Harness type when the target is in the same process
-    RUST,
+    InProcess,
-    /// Harness type when the harness is linked via FFI (e.g C code)
+    /// Harness type when the target is a child process
-    FFI,
+    Child,
 }
 
 /// An observer looking at the backtrace after the harness crashes
 #[allow(clippy::unsafe_derive_deserialize)]
-#[derive(Serialize, Deserialize, Debug, Clone)]
+#[derive(Serialize, Deserialize, Debug)]
-pub struct BacktraceObserver {
+pub struct BacktraceObserver<'a> {
     observer_name: String,
+    hash: OwnedRefMut<'a, Option<u64>>,
     harness_type: HarnessType,
-    hash: Option<u64>,
 }
 
-impl BacktraceObserver {
+impl<'a> BacktraceObserver<'a> {
     /// Creates a new [`BacktraceObserver`] with the given name.
     #[must_use]
-    pub fn new(observer_name: &str, harness_type: HarnessType) -> Self {
+    pub fn new(
+        observer_name: &str,
+        backtrace_hash: &'a mut Option<u64>,
+        harness_type: HarnessType,
+    ) -> Self {
         Self {
             observer_name: observer_name.to_string(),
+            hash: OwnedRefMut::Ref(backtrace_hash),
             harness_type,
-            hash: None,
+        }
     }
 }
 
-    /// Setup the shared memory and store it in [`BACKTRACE_HASH_VALUE`]
+impl<'a> ObserverWithHashField for BacktraceObserver<'a> {
-    pub fn setup_shmem() {
-        let shmem_provider = StdShMemProvider::new();
-        let mut shmem = shmem_provider.unwrap().new_shmem(16).unwrap();
-        shmem.as_mut_slice().fill(0);
-        let boxed_shmem = Box::<StdShMem>::new(shmem);
-        unsafe {
-            BACKTRACE_HASH_VALUE = BacktraceHashValueWrapper::Shmem(boxed_shmem);
-        }
-    }
-
-    /// Init the [`BACKTRACE_HASH_VALUE`] to [`BacktraceHashValueWrapper::StaticVariable`] with `(0.0)`
-    pub fn setup_static_variable() {
-        unsafe {
-            BACKTRACE_HASH_VALUE = BacktraceHashValueWrapper::StaticVariable((0, 0));
-        }
-    }
-
-    /// returns `harness_type` for this [`BacktraceObserver`] instance
-    #[must_use]
-    pub fn harness_type(&self) -> &HarnessType {
-        &self.harness_type
-    }
-}
-
-impl ObserverWithHashField for BacktraceObserver {
     /// Gets the hash value of this observer.
     #[must_use]
     fn hash(&self) -> &Option<u64> {
-        &self.hash
+        self.hash.as_ref()
     }
 
     /// Updates the hash value of this observer.
     fn update_hash(&mut self, hash: u64) {
-        self.hash = Some(hash);
+        *self.hash.as_mut() = Some(hash);
     }
 
     /// Clears the current hash value
     fn clear_hash(&mut self) {
-        self.hash = None;
+        *self.hash.as_mut() = None;
     }
 }
 
-impl Default for BacktraceObserver {
+impl<'a, I, S> Observer<I, S> for BacktraceObserver<'a>
-    fn default() -> Self {
-        Self::new("BacktraceObserver", HarnessType::RUST)
-    }
-}
-
-impl<I, S> Observer<I, S> for BacktraceObserver
 where
     I: Input + Debug,
 {
-    fn post_exec(&mut self, _state: &mut S, input: &I, exit_kind: &ExitKind) -> Result<(), Error> {
+    fn post_exec(&mut self, _state: &mut S, _input: &I, exit_kind: &ExitKind) -> Result<(), Error> {
-        // run if this call resulted after a crash
+        if self.harness_type == HarnessType::InProcess {
             if exit_kind == &ExitKind::Crash {
-            // hash input
+                self.update_hash(collect_backtrace());
-            let mut hasher = DefaultHasher::new();
+            } else {
-            input.hash(&mut hasher);
+                self.clear_hash();
-            let input_hash = hasher.finish();
-            // get last backtrace hash and associated input hash
-            let (bt_hash, current_input_hash) =
-                unsafe { BACKTRACE_HASH_VALUE.get_stacktrace_hash()? };
-            // replace if this is a new input
-            if current_input_hash != input_hash {
-                let bt_hash = collect_backtrace();
-                unsafe { BACKTRACE_HASH_VALUE.store_stacktrace_hash(bt_hash, input_hash) };
             }
-            // update hash field in this observer
-            self.update_hash(bt_hash);
         }
         Ok(())
     }
 
     fn post_exec_child(
         &mut self,
-        state: &mut S,
+        _state: &mut S,
-        input: &I,
+        _input: &I,
         exit_kind: &ExitKind,
     ) -> Result<(), Error> {
-        self.post_exec(state, input, exit_kind)
+        if self.harness_type == HarnessType::Child {
+            if exit_kind == &ExitKind::Crash {
+                self.update_hash(collect_backtrace());
+            } else {
+                self.clear_hash();
+            }
+        }
+        Ok(())
     }
 }
 
-impl Named for BacktraceObserver {
+impl<'a> Named for BacktraceObserver<'a> {
     fn name(&self) -> &str {
         &self.observer_name
     }
 }
 
 /// static variable of ASAN log path
-pub static ASAN_LOG_PATH: &str = "./asanlog";
+pub static ASAN_LOG_PATH: &str = "./asanlog"; // TODO make it unique
 
 /// returns the recommended ASAN runtime flags to capture the backtrace correctly with `log_path` set
 #[must_use]
@@ -287,13 +205,17 @@ impl ASANBacktraceObserver {
 
     /// parse ASAN error output emited by the target command and compute the hash
     pub fn parse_asan_output(&mut self, output: &str) {
-        let mut hasher = AHasher::new_with_keys(0, 0);
+        let mut hash = 0;
         let matcher = Regex::new("\\s*#[0-9]*\\s0x[0-9a-f]*\\sin\\s(.*)").unwrap();
         matcher.captures_iter(output).for_each(|m| {
             let g = m.get(1).unwrap();
-            hasher.write(g.as_str().as_bytes());
+            hash ^= g.as_str().parse::<u64>().unwrap();
+            println!(
+                ">> {} {:#x}",
+                g.as_str(),
+                g.as_str().parse::<u64>().unwrap()
+            );
         });
-        let hash = hasher.finish();
         self.update_hash(hash);
     }
 }