Fix as_object UB discussed in #1748 (#1751)

* Fix as_object UB discussed in #1748 * More cleanup, more less UB * Fix fixes * Added uninit_on_shmem api * clippy * fmt * trying to fix fuzzers, libfuzzer wrapper * Add OwnedRefMit::owned constructor, libfuzzer fix * Some more fixes * Add BacktaceObserver::owned fn * fmt * more fmt
AFLplusplus · Dec 22, 2023 · c93291a · c93291a
1 parent 4e7d2ca
commit c93291a
Show file tree

Hide file tree

Showing 15 changed files with 247 additions and 142 deletions.
diff --git a/fuzzers/backtrace_baby_fuzzers/c_code_with_fork_executor/src/main.rs b/fuzzers/backtrace_baby_fuzzers/c_code_with_fork_executor/src/main.rs
@@ -18,8 +18,9 @@ use libafl::{
 };
 use libafl_bolts::{
     current_nanos,
+    ownedref::OwnedRefMut,
     rands::StdRand,
-    shmem::{ShMem, ShMemProvider, StdShMemProvider},
+    shmem::{ShMemProvider, StdShMemProvider},
     tuples::tuple_list,
     AsSlice,
 };
@@ -47,10 +48,10 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = unsafe { ConstMapObserver::<u8, 3>::from_mut_ptr("signals", map_ptr) };
     // Create a stacktrace observer
-    let mut bt = shmem_provider.new_shmem_object::<Option<u64>>().unwrap();
+    let mut bt = shmem_provider.new_on_shmem::<Option<u64>>(None).unwrap();
     let bt_observer = BacktraceObserver::new(
         "BacktraceObserver",
-        unsafe { bt.as_object_mut::<Option<u64>>() },
+        unsafe { OwnedRefMut::from_shmem(&mut bt) },
         libafl::observers::HarnessType::Child,
     );
 

diff --git a/fuzzers/backtrace_baby_fuzzers/c_code_with_inprocess_executor/src/main.rs b/fuzzers/backtrace_baby_fuzzers/c_code_with_inprocess_executor/src/main.rs
@@ -37,10 +37,8 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = unsafe { ConstMapObserver::<u8, 3>::from_mut_ptr("signals", array_ptr) };
     // Create a stacktrace observer
-    let mut bt = None;
-    let bt_observer = BacktraceObserver::new(
+    let bt_observer = BacktraceObserver::owned(
         "BacktraceObserver",
-        &mut bt,
         libafl::observers::HarnessType::InProcess,
     );
 

diff --git a/fuzzers/backtrace_baby_fuzzers/rust_code_with_fork_executor/src/main.rs b/fuzzers/backtrace_baby_fuzzers/rust_code_with_fork_executor/src/main.rs
@@ -20,6 +20,7 @@ use libafl::{
 };
 use libafl_bolts::{
     current_nanos,
+    ownedref::OwnedRefMut,
     rands::StdRand,
     shmem::{unix_shmem, ShMem, ShMemProvider},
     tuples::tuple_list,
@@ -32,7 +33,7 @@ pub fn main() {
     let mut signals = shmem_provider.new_shmem(16).unwrap();
     let signals_len = signals.len();
     let signals_ptr = signals.as_mut_slice().as_mut_ptr();
-    let mut bt = shmem_provider.new_shmem_object::<Option<u64>>().unwrap();
+    let mut bt = shmem_provider.new_on_shmem::<Option<u64>>(None).unwrap();
 
     let signals_set = |idx: usize| {
         unsafe { write(signals_ptr.add(idx), 1) };
@@ -69,7 +70,7 @@ pub fn main() {
     // Create a stacktrace observer
     let bt_observer = BacktraceObserver::new(
         "BacktraceObserver",
-        unsafe { bt.as_object_mut::<Option<u64>>() },
+        unsafe { OwnedRefMut::from_shmem(&mut bt) },
         libafl::observers::HarnessType::Child,
     );
 

diff --git a/fuzzers/backtrace_baby_fuzzers/rust_code_with_inprocess_executor/src/main.rs b/fuzzers/backtrace_baby_fuzzers/rust_code_with_inprocess_executor/src/main.rs
@@ -61,10 +61,8 @@ pub fn main() {
     // Create an observation channel using the signals map
     let observer = unsafe { StdMapObserver::from_mut_ptr("signals", SIGNALS_PTR, SIGNALS.len()) };
     // Create a stacktrace observer to add the observers tuple
-    let mut bt = None;
-    let bt_observer = BacktraceObserver::new(
+    let bt_observer = BacktraceObserver::owned(
         "BacktraceObserver",
-        &mut bt,
         libafl::observers::HarnessType::InProcess,
     );
 

diff --git a/fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs b/fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs
@@ -209,9 +209,7 @@ fn fuzz(
     let edges = edges_shmem.as_mut_slice();
     unsafe { EDGES_MAP_PTR = edges.as_mut_ptr() };
 
-    let mut cmp_shmem = shmem_provider
-        .new_shmem(core::mem::size_of::<CmpLogMap>())
-        .unwrap();
+    let mut cmp_shmem = shmem_provider.uninit_on_shmem::<CmpLogMap>().unwrap();
     let cmplog = cmp_shmem.as_mut_slice();
 
     // Beginning of a page should be properly aligned.

diff --git a/fuzzers/fuzzbench_forkserver/src/main.rs b/fuzzers/fuzzbench_forkserver/src/main.rs
@@ -34,6 +34,7 @@ use libafl::{
 };
 use libafl_bolts::{
     current_nanos, current_time,
+    ownedref::OwnedRefMut,
     rands::StdRand,
     shmem::{ShMem, ShMemProvider, UnixShMemProvider},
     tuples::{tuple_list, Merge},
@@ -204,6 +205,7 @@ pub fn main() {
 }
 
 /// The actual fuzzer
+#[allow(clippy::too_many_arguments)]
 fn fuzz(
     corpus_dir: PathBuf,
     objective_dir: PathBuf,
@@ -340,12 +342,10 @@ fn fuzz(
 
     if let Some(exec) = &cmplog_exec {
         // The cmplog map shared between observer and executor
-        let mut cmplog_shmem = shmem_provider
-            .new_shmem(core::mem::size_of::<AFLppCmpLogMap>())
-            .unwrap();
+        let mut cmplog_shmem = shmem_provider.uninit_on_shmem::<AFLppCmpLogMap>().unwrap();
         // let the forkserver know the shmid
         cmplog_shmem.write_to_env("__AFL_CMPLOG_SHM_ID").unwrap();
-        let cmpmap = unsafe { cmplog_shmem.as_object_mut::<AFLppCmpLogMap>() };
+        let cmpmap = unsafe { OwnedRefMut::<AFLppCmpLogMap>::from_shmem(&mut cmplog_shmem) };
 
         let cmplog_observer = StdCmpValuesObserver::new("cmplog", cmpmap, true);
 

diff --git a/fuzzers/fuzzbench_forkserver_cmplog/src/main.rs b/fuzzers/fuzzbench_forkserver_cmplog/src/main.rs
@@ -34,6 +34,7 @@ use libafl::{
 };
 use libafl_bolts::{
     current_nanos, current_time,
+    ownedref::OwnedRefMut,
     rands::StdRand,
     shmem::{ShMem, ShMemProvider, UnixShMemProvider},
     tuples::{tuple_list, Merge},
@@ -207,6 +208,7 @@ pub fn main() {
 }
 
 /// The actual fuzzer
+#[allow(clippy::too_many_arguments)]
 fn fuzz(
     corpus_dir: PathBuf,
     objective_dir: PathBuf,
@@ -344,12 +346,10 @@ fn fuzz(
 
     if let Some(exec) = &cmplog_exec {
         // The cmplog map shared between observer and executor
-        let mut cmplog_shmem = shmem_provider
-            .new_shmem(core::mem::size_of::<AFLppCmpLogMap>())
-            .unwrap();
+        let mut cmplog_shmem = shmem_provider.uninit_on_shmem::<AFLppCmpLogMap>().unwrap();
         // let the forkserver know the shmid
         cmplog_shmem.write_to_env("__AFL_CMPLOG_SHM_ID").unwrap();
-        let cmpmap = unsafe { cmplog_shmem.as_object_mut::<AFLppCmpLogMap>() };
+        let cmpmap = unsafe { OwnedRefMut::from_shmem(&mut cmplog_shmem) };
 
         let cmplog_observer = AFLppCmpLogObserver::new("cmplog", cmpmap, true);
 

diff --git a/libafl/src/observers/cmp.rs b/libafl/src/observers/cmp.rs
@@ -333,11 +333,11 @@ where
 {
     /// Creates a new [`StdCmpObserver`] with the given name and map.
     #[must_use]
-    pub fn new(name: &'static str, map: &'a mut CM, add_meta: bool) -> Self {
+    pub fn new(name: &'static str, map: OwnedRefMut<'a, CM>, add_meta: bool) -> Self {
         Self {
             name: name.to_string(),
             size: None,
-            cmp_map: OwnedRefMut::Ref(map),
+            cmp_map: map,
             add_meta,
             data: M::Data::default(),
             phantom: PhantomData,
@@ -347,11 +347,16 @@ where
     /// Creates a new [`StdCmpObserver`] with the given name, map, and auxiliary data used to
     /// populate metadata
     #[must_use]
-    pub fn with_data(name: &'static str, map: &'a mut CM, add_meta: bool, data: M::Data) -> Self {
+    pub fn with_data(
+        name: &'static str,
+        cmp_map: OwnedRefMut<'a, CM>,
+        add_meta: bool,
+        data: M::Data,
+    ) -> Self {
         Self {
             name: name.to_string(),
             size: None,
-            cmp_map: OwnedRefMut::Ref(map),
+            cmp_map,
             add_meta,
             data,
             phantom: PhantomData,
@@ -362,14 +367,14 @@ where
     #[must_use]
     pub fn with_size(
         name: &'static str,
-        map: &'a mut CM,
+        cmp_map: OwnedRefMut<'a, CM>,
         add_meta: bool,
-        size: &'a mut usize,
+        size: OwnedRefMut<'a, usize>,
     ) -> Self {
         Self {
             name: name.to_string(),
-            size: Some(OwnedRefMut::Ref(size)),
-            cmp_map: OwnedRefMut::Ref(map),
+            size: Some(size),
+            cmp_map,
             add_meta,
             data: M::Data::default(),
             phantom: PhantomData,
@@ -381,15 +386,15 @@ where
     #[must_use]
     pub fn with_size_data(
         name: &'static str,
-        map: &'a mut CM,
+        cmp_map: OwnedRefMut<'a, CM>,
         add_meta: bool,
         data: M::Data,
-        size: &'a mut usize,
+        size: OwnedRefMut<'a, usize>,
     ) -> Self {
         Self {
             name: name.to_string(),
-            size: Some(OwnedRefMut::Ref(size)),
-            cmp_map: OwnedRefMut::Ref(map),
+            size: Some(size),
+            cmp_map,
             add_meta,
             data,
             phantom: PhantomData,

diff --git a/libafl/src/observers/stacktrace.rs b/libafl/src/observers/stacktrace.rs
@@ -127,12 +127,12 @@ impl<'a> BacktraceObserver<'a> {
     #[must_use]
     pub fn new(
         observer_name: &str,
-        backtrace_hash: &'a mut Option<u64>,
+        backtrace_hash: OwnedRefMut<'a, Option<u64>>,
         harness_type: HarnessType,
     ) -> Self {
         Self {
             observer_name: observer_name.to_string(),
-            hash: OwnedRefMut::Ref(backtrace_hash),
+            hash: backtrace_hash,
             harness_type,
         }
     }
@@ -142,17 +142,23 @@ impl<'a> BacktraceObserver<'a> {
     #[must_use]
     pub fn new(
         observer_name: &str,
-        backtrace_hash: &'a mut Option<u64>,
+        backtrace_hash: OwnedRefMut<'a, Option<u64>>,
         harness_type: HarnessType,
     ) -> Self {
         init_ignored_frames!("rust", "cpp", "go");
         Self {
             observer_name: observer_name.to_string(),
-            hash: OwnedRefMut::Ref(backtrace_hash),
+            hash: backtrace_hash,
             harness_type,
         }
     }
 
+    /// Creates a new [`BacktraceObserver`] with the given name, owning a new `backtrace_hash` variable.
+    #[must_use]
+    pub fn owned(observer_name: &str, harness_type: HarnessType) -> Self {
+        Self::new(observer_name, OwnedRefMut::owned(None), harness_type)
+    }
+
     /// Updates the hash value of this observer.
     fn update_hash(&mut self, hash: u64) {
         *self.hash.as_mut() = Some(hash);

diff --git a/libafl/src/observers/value.rs b/libafl/src/observers/value.rs
@@ -40,10 +40,10 @@ where
 {
     /// Creates a new [`ValueObserver`] with the given name.
     #[must_use]
-    pub fn new(name: &'static str, value: &'a T) -> Self {
+    pub fn new(name: &'static str, value: OwnedRef<'a, T>) -> Self {
         Self {
             name: name.to_string(),
-            value: OwnedRef::Ref(value),
+            value,
         }
     }
 
@@ -65,6 +65,7 @@ where
         T: Clone,
     {
         match self.value {
+            OwnedRef::RefRaw(r, _) => unsafe { (*r).clone() },
             OwnedRef::Ref(r) => r.clone(),
             OwnedRef::Owned(v) => *v,
         }
@@ -119,10 +120,10 @@ where
 {
     /// Creates a new [`RefCellValueObserver`] with the given name.
     #[must_use]
-    pub fn new(name: &'static str, value: &'a RefCell<T>) -> Self {
+    pub fn new(name: &'static str, value: OwnedRef<'a, RefCell<T>>) -> Self {
         Self {
             name: name.to_string(),
-            value: OwnedRef::Ref(value),
+            value,
         }
     }
 
@@ -147,6 +148,7 @@ where
         T: Clone,
     {
         match self.value {
+            OwnedRef::RefRaw(r, _) => unsafe { (*r).borrow().deref().clone() },
             OwnedRef::Ref(r) => r.borrow().deref().clone(),
             OwnedRef::Owned(v) => v.borrow().clone(),
         }