diff --git a/fuzzers/frida_gdiplus/src/fuzzer.rs b/fuzzers/frida_gdiplus/src/fuzzer.rs index 665ee5b5eb..a226311782 100644 --- a/fuzzers/frida_gdiplus/src/fuzzer.rs +++ b/fuzzers/frida_gdiplus/src/fuzzer.rs @@ -17,8 +17,8 @@ use libafl::{ corpus::{CachedOnDiskCorpus, Corpus, OnDiskCorpus}, events::{launcher::Launcher, llmp::LlmpRestartingEventManager, EventConfig}, executors::{inprocess::InProcessExecutor, ExitKind, ShadowExecutor}, - feedback_and_fast, feedback_or, feedback_or_fast, - feedbacks::{ConstFeedback, CrashFeedback, MaxMapFeedback, TimeFeedback, TimeoutFeedback}, + feedback_or, feedback_or_fast, + feedbacks::{CrashFeedback, MaxMapFeedback, TimeFeedback, TimeoutFeedback}, fuzzer::{Fuzzer, StdFuzzer}, inputs::{BytesInput, HasTargetBytes}, monitors::MultiMonitor, @@ -32,6 +32,8 @@ use libafl::{ state::{HasCorpus, HasMetadata, StdState}, Error, }; +#[cfg(unix)] +use libafl::{feedback_and_fast, feedbacks::ConstFeedback}; use libafl_bolts::{ cli::{parse_args, FuzzerOptions}, current_nanos, diff --git a/libafl_frida/src/executor.rs b/libafl_frida/src/executor.rs index 3943b85e1f..8445322cf8 100644 --- a/libafl_frida/src/executor.rs +++ b/libafl_frida/src/executor.rs @@ -1,5 +1,5 @@ use core::fmt::{self, Debug, Formatter}; -use std::{ffi::c_void, marker::PhantomData, process}; +use std::{ffi::c_void, marker::PhantomData}; use frida_gum::{ stalker::{NoneEventSink, Stalker}, @@ -35,7 +35,7 @@ where { base: InProcessExecutor<'a, H, OT, S>, // thread_id for the Stalker - thread_id: u32, + thread_id: Option, /// Frida's dynamic rewriting engine stalker: Stalker<'a>, /// User provided callback for instrumentation @@ -87,11 +87,15 @@ where } else { self.followed = true; let transformer = self.helper.transformer(); - self.stalker.follow::( - self.thread_id.try_into().unwrap(), - transformer, - None, - ); + if let Some(thread_id) = self.thread_id { + self.stalker.follow::( + thread_id.try_into().unwrap(), + transformer, + None, + ); + } else { + self.stalker.follow_me::(transformer, None); + } } } let res = self.base.run_target(fuzzer, state, mgr, input); @@ -162,7 +166,7 @@ where base: InProcessExecutor<'a, H, OT, S>, helper: &'c mut FridaInstrumentationHelper<'b, RT>, ) -> Self { - Self::on_thread(gum, base, helper, process::id()) + Self::_on_thread(gum, base, helper, None) } /// Creates a new [`FridaInProcessExecutor`] tracking the given `thread_id`. @@ -171,6 +175,16 @@ where base: InProcessExecutor<'a, H, OT, S>, helper: &'c mut FridaInstrumentationHelper<'b, RT>, thread_id: u32, + ) -> Self { + Self::_on_thread(gum, base, helper, Some(thread_id)) + } + + /// Creates a new [`FridaInProcessExecutor`] tracking the given `thread_id`, of `thread_id` is provided. + fn _on_thread( + gum: &'a Gum, + base: InProcessExecutor<'a, H, OT, S>, + helper: &'c mut FridaInstrumentationHelper<'b, RT>, + thread_id: Option, ) -> Self { let mut stalker = Stalker::new(gum); // Include the current module (the fuzzer) in stalked ranges. We clone the ranges so that diff --git a/libafl_frida/src/pthread_hook.rs b/libafl_frida/src/pthread_hook.rs index a9f17a9471..337110dcee 100644 --- a/libafl_frida/src/pthread_hook.rs +++ b/libafl_frida/src/pthread_hook.rs @@ -1,5 +1,4 @@ use std::{ - cell::UnsafeCell, convert::{TryFrom, TryInto}, sync::RwLock, }; @@ -21,49 +20,43 @@ type pthread_introspection_hook_t = extern "C" fn( ); extern "C" { - fn pthread_introspection_hook_install( - hook: *const libc::c_void, - ) -> pthread_introspection_hook_t; + fn pthread_introspection_hook_install(hook: *const libc::c_void) -> *const libc::c_void; } -struct PreviousHook(UnsafeCell>); +struct PreviousHook(*const pthread_introspection_hook_t); impl PreviousHook { /// Dispatch to the previous hook, if it is set. - pub fn dispatch( + pub unsafe fn dispatch( &self, event: libc::c_uint, thread: libc::pthread_t, addr: *const libc::c_void, size: libc::size_t, ) { - let inner = unsafe { *self.0.get() }; - if inner.is_none() { + let inner = self.0; + if inner.is_null() { return; } - let inner = inner.unwrap(); - inner(event, thread, addr, size); + unsafe { (*inner)(event, thread, addr, size) }; } /// Set the previous hook. - pub fn set(&self, hook: pthread_introspection_hook_t) { - unsafe { - *self.0.get() = Some(hook); - } + pub fn set(&mut self, hook: *const pthread_introspection_hook_t) { + self.0 = hook; } /// Ensure the previous hook is installed again. - pub fn reset(&self) { - let inner = unsafe { *self.0.get() }; - if inner.is_none() { + pub fn reset(&mut self) { + let inner = self.0; + if inner.is_null() { unsafe { pthread_introspection_hook_install(std::ptr::null()); } return; } - let inner = inner.unwrap(); unsafe { - *self.0.get() = None; + self.0 = std::ptr::null(); pthread_introspection_hook_install(inner as *const libc::c_void); } } @@ -74,7 +67,7 @@ impl PreviousHook { unsafe impl Sync for PreviousHook {} #[allow(non_upper_case_globals)] -static PREVIOUS_HOOK: PreviousHook = PreviousHook(UnsafeCell::new(None)); +static mut PREVIOUS_HOOK: PreviousHook = PreviousHook(std::ptr::null()); static CURRENT_HOOK: RwLock> = RwLock::new(None); @@ -87,7 +80,7 @@ extern "C" fn pthread_introspection_hook( if let Some(ref hook) = *CURRENT_HOOK.read().unwrap() { hook(event.try_into().unwrap(), thread, addr, size); } - PREVIOUS_HOOK.dispatch(event, thread, addr, size); + unsafe { PREVIOUS_HOOK.dispatch(event, thread, addr, size) }; } /// Closure type for `pthread_introspection` hooks. @@ -153,7 +146,10 @@ impl From for libc::c_uint { /// thread id=0x16bf67000 event=Terminate addr=0x16bd60000 size=208000 /// thread id=0x16bf67000 event=Destroy addr=0x16bf67000 size=4000 /// ``` -pub fn install(hook: H) +/// +/// # Safety +/// Potential data race when if called at the same time as `install` or `reset` from another thread +pub unsafe fn install(hook: H) where H: Fn(EventType, libc::pthread_t, *const libc::c_void, libc::size_t) + Send + Sync + 'static, { @@ -165,9 +161,10 @@ where }; // Allow because we're sure this isn't from a different code generation unit. - #[allow(clippy::fn_address_comparisons, clippy::fn_null_check)] - if !(prev as *const libc::c_void).is_null() && prev != pthread_introspection_hook { - PREVIOUS_HOOK.set(prev); + if !(prev).is_null() && prev != pthread_introspection_hook as *const libc::c_void { + unsafe { + PREVIOUS_HOOK.set(prev as *const pthread_introspection_hook_t); + } } } @@ -179,8 +176,11 @@ where ///# use std::thread; /// pthread_hook::reset(); /// ``` -pub fn reset() { - PREVIOUS_HOOK.reset(); +/// +/// # Safety +/// Potential data race when if called at the same time as `install` or `reset` from another thread +pub unsafe fn reset() { + unsafe { PREVIOUS_HOOK.reset() }; } /// The following tests fail if they are not run sequentially. @@ -204,7 +204,7 @@ mod test { }); thread::sleep(Duration::from_millis(50)); - super::reset(); + unsafe { super::reset() }; assert!(!*triggered.lock().unwrap()); } @@ -214,19 +214,21 @@ mod test { let triggered: Arc> = Arc::new(Mutex::new(false)); let inner_triggered = triggered.clone(); - super::install(move |event, _, _, _| { - if event == super::EventType::Create { - let mut triggered = inner_triggered.lock().unwrap(); - *triggered = true; - } - }); + unsafe { + super::install(move |event, _, _, _| { + if event == super::EventType::Create { + let mut triggered = inner_triggered.lock().unwrap(); + *triggered = true; + } + }) + }; thread::spawn(|| { thread::sleep(Duration::from_millis(1)); }); thread::sleep(Duration::from_millis(50)); - super::reset(); + unsafe { super::reset() }; assert!(*triggered.lock().unwrap()); } @@ -236,19 +238,21 @@ mod test { let triggered: Arc> = Arc::new(Mutex::new(false)); let inner_triggered = triggered.clone(); - super::install(move |event, _, _, _| { - if event == super::EventType::Start { - let mut triggered = inner_triggered.lock().unwrap(); - *triggered = true; - } - }); + unsafe { + super::install(move |event, _, _, _| { + if event == super::EventType::Start { + let mut triggered = inner_triggered.lock().unwrap(); + *triggered = true; + } + }) + }; thread::spawn(|| { thread::sleep(Duration::from_millis(1)); }); thread::sleep(Duration::from_millis(50)); - super::reset(); + unsafe { super::reset() }; assert!(*triggered.lock().unwrap()); } @@ -258,14 +262,16 @@ mod test { let triggered: Arc> = Arc::new(Mutex::new(false)); let inner_triggered = triggered.clone(); - super::install(move |event, _, _, _| { - if event == super::EventType::Start { - let mut triggered = inner_triggered.lock().unwrap(); - *triggered = true; - } - }); + unsafe { + super::install(move |event, _, _, _| { + if event == super::EventType::Start { + let mut triggered = inner_triggered.lock().unwrap(); + *triggered = true; + } + }) + }; - super::reset(); + unsafe { super::reset() }; thread::spawn(|| { thread::sleep(Duration::from_millis(1));