From 454176427b590b7e9d4a47b7f584ac7122e2aa44 Mon Sep 17 00:00:00 2001 From: "Dongjia \"toka\" Zhang" Date: Sat, 8 Jun 2024 20:32:40 +0200 Subject: [PATCH] Windows clippy (#2295) * add * real one * fuck * abc * def * ghi * jkl * fix --------- Co-authored-by: Romain Malmain --- .github/workflows/build_and_test.yml | 7 ++- libafl/Cargo.toml | 3 +- libafl/src/corpus/mod.rs | 4 +- libafl/src/events/launcher.rs | 32 ++++++++++---- libafl/src/events/tcp.rs | 4 +- libafl/src/stages/mod.rs | 6 +-- libafl/src/stages/tracing.rs | 5 ++- libafl_bolts/src/os/windows_exceptions.rs | 5 ++- libafl_frida/src/asan/asan_rt.rs | 1 + libafl_frida/src/asan/hook_funcs.rs | 40 +++++++---------- libafl_frida/src/executor.rs | 6 +-- libafl_frida/src/lib.rs | 5 ++- libafl_frida/src/windows_hooks.rs | 50 +++++++++++----------- libafl_qemu/build_linux.rs | 4 +- libafl_qemu/libafl_qemu_build/src/lib.rs | 10 ++--- libafl_qemu/libafl_qemu_sys/build_linux.rs | 4 +- libafl_targets/src/windows_asan.rs | 4 +- scripts/clippy.ps1 | 2 +- 18 files changed, 101 insertions(+), 91 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index f0d70ba47e..20afa93a11 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -479,10 +479,9 @@ jobs: toolchain: stable - uses: actions/checkout@v3 - uses: Swatinem/rust-cache@v2 - - name: Run clippy - uses: actions-rs/cargo@v1 - with: - command: clippy + - name: Run real clippy, not the fake one + shell: pwsh + run: .\scripts\clippy.ps1 macos: runs-on: macOS-latest diff --git a/libafl/Cargo.toml b/libafl/Cargo.toml index 5a1b2ceccd..6b26cff722 100644 --- a/libafl/Cargo.toml +++ b/libafl/Cargo.toml @@ -175,8 +175,6 @@ tokio = { version = "1.28.1", optional = true, features = ["sync", "net", "rt", wait-timeout = { version = "0.2", optional = true } # used by CommandExecutor to wait for child process -z3 = { version = "0.12.0", optional = true } # for concolic mutation - concat-idents = { version = "1.1.3", optional = true } libcasr = { version = "2.7", optional = true } @@ -199,6 +197,7 @@ document-features = { version = "0.2", optional = true } [target.'cfg(unix)'.dependencies] libc = "0.2" # For (*nix) libc +z3 = { version = "0.12.0", optional = true } # for concolic mutation [target.'cfg(windows)'.dependencies] windows = { version = "0.51.1", features = ["Win32_Foundation", "Win32_System_Threading", "Win32_System_Diagnostics_Debug", "Win32_System_Kernel", "Win32_System_Memory", "Win32_Security", "Win32_System_SystemInformation"] } diff --git a/libafl/src/corpus/mod.rs b/libafl/src/corpus/mod.rs index edf47fe5b5..c3b1f5f546 100644 --- a/libafl/src/corpus/mod.rs +++ b/libafl/src/corpus/mod.rs @@ -21,12 +21,12 @@ pub mod cached; #[cfg(feature = "std")] pub use cached::CachedOnDiskCorpus; -#[cfg(feature = "cmin")] +#[cfg(all(feature = "cmin", unix))] pub mod minimizer; use core::{cell::RefCell, fmt}; pub mod nop; -#[cfg(feature = "cmin")] +#[cfg(all(feature = "cmin", unix))] pub use minimizer::*; pub use nop::NopCorpus; use serde::{Deserialize, Serialize}; diff --git a/libafl/src/events/launcher.rs b/libafl/src/events/launcher.rs index 727eaab732..5a22261ffb 100644 --- a/libafl/src/events/launcher.rs +++ b/libafl/src/events/launcher.rs @@ -35,7 +35,14 @@ use libafl_bolts::os::dup2; #[cfg(all(feature = "std", any(windows, not(feature = "fork"))))] use libafl_bolts::os::startable_self; #[cfg(feature = "adaptive_serialization")] -use libafl_bolts::tuples::{Handle, Handled}; +use libafl_bolts::tuples::Handle; +#[cfg(all( + unix, + feature = "std", + feature = "fork", + feature = "adaptive_serialization" +))] +use libafl_bolts::tuples::Handled; #[cfg(all(unix, feature = "std", feature = "fork"))] use libafl_bolts::{ core_affinity::get_core_ids, @@ -342,7 +349,8 @@ where Ok(core_conf) => { let core_id = core_conf.parse()?; // the actual client. do the fuzzing - let (state, mgr) = RestartingMgr::::builder() + + let builder = RestartingMgr::::builder() .shmem_provider(self.shmem_provider.clone()) .broker_port(self.broker_port) .kind(ManagerKind::Client { @@ -350,9 +358,12 @@ where }) .configuration(self.configuration) .serialize_state(self.serialize_state) - .hooks(hooks) - .build() - .launch()?; + .hooks(hooks); + + #[cfg(feature = "adaptive_serialization")] + let builder = builder.time_ref(self.time_ref.clone()); + + let (state, mgr) = builder.build().launch()?; return (self.run_client.take().unwrap())(state, mgr, CoreId(core_id)); } @@ -433,7 +444,7 @@ where #[cfg(feature = "std")] log::info!("I am broker!!."); - RestartingMgr::::builder() + let builder = RestartingMgr::::builder() .shmem_provider(self.shmem_provider.clone()) .monitor(Some(self.monitor.clone())) .broker_port(self.broker_port) @@ -442,9 +453,12 @@ where .exit_cleanly_after(Some(NonZeroUsize::try_from(self.cores.ids.len()).unwrap())) .configuration(self.configuration) .serialize_state(self.serialize_state) - .hooks(hooks) - .build() - .launch()?; + .hooks(hooks); + + #[cfg(feature = "adaptive_serialization")] + let builder = builder.time_ref(self.time_ref.clone()); + + builder.build().launch()?; //broker exited. kill all clients. for handle in &mut handles { diff --git a/libafl/src/events/tcp.rs b/libafl/src/events/tcp.rs index fd7cd74072..eb0010241f 100644 --- a/libafl/src/events/tcp.rs +++ b/libafl/src/events/tcp.rs @@ -24,6 +24,8 @@ use libafl_bolts::core_affinity::CoreId; use libafl_bolts::os::startable_self; #[cfg(all(unix, feature = "std", not(miri)))] use libafl_bolts::os::unix_signals::setup_signal_handler; +#[cfg(feature = "std")] +use libafl_bolts::os::CTRL_C_EXIT; #[cfg(all(feature = "std", feature = "fork", unix))] use libafl_bolts::os::{fork, ForkResult}; use libafl_bolts::{shmem::ShMemProvider, tuples::tuple_list, ClientId}; @@ -1278,7 +1280,7 @@ where compiler_fence(Ordering::SeqCst); - if child_status == crate::events::CTRL_C_EXIT || staterestorer.wants_to_exit() { + if child_status == CTRL_C_EXIT || staterestorer.wants_to_exit() { return Err(Error::shutting_down()); } diff --git a/libafl/src/stages/mod.rs b/libafl/src/stages/mod.rs index d1d6e2f70e..be65359111 100644 --- a/libafl/src/stages/mod.rs +++ b/libafl/src/stages/mod.rs @@ -9,9 +9,9 @@ use core::{fmt, marker::PhantomData}; pub use calibrate::CalibrationStage; pub use colorization::*; -#[cfg(feature = "std")] +#[cfg(all(feature = "std", unix))] pub use concolic::ConcolicTracingStage; -#[cfg(all(feature = "std", feature = "concolic_mutation"))] +#[cfg(all(feature = "std", feature = "concolic_mutation", unix))] pub use concolic::SimpleConcolicMutationalStage; #[cfg(feature = "std")] pub use dump::*; @@ -58,7 +58,7 @@ pub mod tmin; pub mod calibrate; pub mod colorization; -#[cfg(feature = "std")] +#[cfg(all(feature = "std", unix))] pub mod concolic; #[cfg(feature = "std")] pub mod dump; diff --git a/libafl/src/stages/tracing.rs b/libafl/src/stages/tracing.rs index 14127ac69e..052adc98b2 100644 --- a/libafl/src/stages/tracing.rs +++ b/libafl/src/stages/tracing.rs @@ -40,9 +40,10 @@ where EM: UsesState::State>, Z: UsesState::State>, { + #[allow(rustdoc::broken_intra_doc_links)] /// Perform tracing on the given `CorpusId`. Useful for if wrapping [`TracingStage`] with your - /// own stage and you need to manage [`super::NestedStageRestartHelper`] differently; see - /// [`super::ConcolicTracingStage`]'s implementation as an example of usage. + /// own stage and you need to manage [`super::NestedStageRestartHelper`] differently + /// see [`super::ConcolicTracingStage`]'s implementation as an example of usage. pub fn trace( &mut self, fuzzer: &mut Z, diff --git a/libafl_bolts/src/os/windows_exceptions.rs b/libafl_bolts/src/os/windows_exceptions.rs index 04b8d2cc12..867b4d6917 100644 --- a/libafl_bolts/src/os/windows_exceptions.rs +++ b/libafl_bolts/src/os/windows_exceptions.rs @@ -562,7 +562,10 @@ pub unsafe fn setup_exception_handler(handler: *mut T) -> // See https://github.com/AFLplusplus/LibAFL/pull/403 AddVectoredExceptionHandler( 0, - Some(core::mem::transmute(handle_exception as *const c_void)), + Some(core::mem::transmute::< + *const core::ffi::c_void, + unsafe extern "system" fn(*mut EXCEPTION_POINTERS) -> i32, + >(handle_exception as *const c_void)), ); Ok(()) } diff --git a/libafl_frida/src/asan/asan_rt.rs b/libafl_frida/src/asan/asan_rt.rs index 02a238874d..e2e745cc4a 100644 --- a/libafl_frida/src/asan/asan_rt.rs +++ b/libafl_frida/src/asan/asan_rt.rs @@ -942,6 +942,7 @@ impl AsanRuntime { let cpp_libs = ["libc++.1.dylib", "libc++abi.dylib", "libsystem_c.dylib"]; */ + #[cfg(any(target_os = "linux", target_vendor = "apple"))] macro_rules! hook_cpp { ($libname:literal, $lib_ident:ident) => { log::info!("Hooking c++ functions in {}", $libname); diff --git a/libafl_frida/src/asan/hook_funcs.rs b/libafl_frida/src/asan/hook_funcs.rs index e3a1786aaf..de1293aa92 100644 --- a/libafl_frida/src/asan/hook_funcs.rs +++ b/libafl_frida/src/asan/hook_funcs.rs @@ -11,7 +11,12 @@ use crate::{ errors::{AsanError, AsanErrors}, }, }; - +extern "system" { + fn memcpy(dst: *mut c_void, src: *const c_void, size: usize) -> *mut c_void; +} +extern "system" { + fn memset(s: *mut c_void, c: i32, n: usize) -> *mut c_void; +} #[allow(clippy::not_unsafe_ptr_arg_deref)] impl AsanRuntime { #[inline] @@ -193,14 +198,11 @@ impl AsanRuntime { let ret = unsafe { allocator.alloc(size, 8) }; if flags & 8 == 8 { - extern "system" { - fn memset(s: *mut c_void, c: i32, n: usize) -> *mut c_void; - } unsafe { memset(ret, 0, size); } } - if flags & 4 == 4 && ret == std::ptr::null_mut() { + if flags & 4 == 4 && ret.is_null() { unimplemented!(); } ret @@ -219,14 +221,11 @@ impl AsanRuntime { let ret = unsafe { allocator.alloc(size, 8) }; if flags & 8 == 8 { - extern "system" { - fn memset(s: *mut c_void, c: i32, n: usize) -> *mut c_void; - } unsafe { memset(ret, 0, size); } } - if flags & 4 == 4 && ret == std::ptr::null_mut() { + if flags & 4 == 4 && ret.is_null() { unimplemented!(); } ret @@ -253,10 +252,8 @@ impl AsanRuntime { } let ret = unsafe { let ret = allocator.alloc(size, 8); - extern "system" { - fn memcpy(dst: *mut c_void, src: *const c_void, size: usize) -> *mut c_void; - } - memcpy(ret as *mut c_void, ptr, allocator.get_usable_size(ptr)); + + memcpy(ret, ptr, allocator.get_usable_size(ptr)); allocator.release(ptr); ret }; @@ -269,7 +266,7 @@ impl AsanRuntime { memset(ret, 0, size); } } - if flags & 4 == 4 && ret == std::ptr::null_mut() { + if flags & 4 == 4 && ret.is_null() { unimplemented!(); } if flags & 0x10 == 0x10 && ret != ptr { @@ -300,10 +297,8 @@ impl AsanRuntime { } let ret = unsafe { let ret = allocator.alloc(size, 8); - extern "system" { - fn memcpy(dst: *mut c_void, src: *const c_void, size: usize) -> *mut c_void; - } - memcpy(ret as *mut c_void, ptr, allocator.get_usable_size(ptr)); + + memcpy(ret, ptr, allocator.get_usable_size(ptr)); allocator.release(ptr); ret }; @@ -316,7 +311,7 @@ impl AsanRuntime { memset(ret, 0, size); } } - if flags & 4 == 4 && ret == std::ptr::null_mut() { + if flags & 4 == 4 && ret.is_null() { unimplemented!(); } if flags & 0x10 == 0x10 && ret != ptr { @@ -453,9 +448,6 @@ impl AsanRuntime { let ret = unsafe { self.allocator_mut().alloc(size, 8) }; if flags & 0x40 == 0x40 { - extern "system" { - fn memset(s: *mut c_void, c: i32, n: usize) -> *mut c_void; - } unsafe { memset(ret, 0, size); } @@ -473,7 +465,7 @@ impl AsanRuntime { ) -> *mut c_void { unsafe { let ret = self.allocator_mut().alloc(size, 0x8); - if mem != std::ptr::null_mut() && ret != std::ptr::null_mut() { + if !mem.is_null() && !ret.is_null() { let old_size = self.allocator_mut().get_usable_size(mem); let copy_size = if size < old_size { size } else { old_size }; (mem as *mut u8).copy_to(ret as *mut u8, copy_size); @@ -603,7 +595,7 @@ impl AsanRuntime { ) -> *mut c_void { unsafe { let ret = self.allocator_mut().alloc(size, 0x8); - if mem != std::ptr::null_mut() && ret != std::ptr::null_mut() { + if !mem.is_null() && !ret.is_null() { let old_size = self.allocator_mut().get_usable_size(mem); let copy_size = if size < old_size { size } else { old_size }; (mem as *mut u8).copy_to(ret as *mut u8, copy_size); diff --git a/libafl_frida/src/executor.rs b/libafl_frida/src/executor.rs index 78488d38f3..0a4afb9370 100644 --- a/libafl_frida/src/executor.rs +++ b/libafl_frida/src/executor.rs @@ -1,6 +1,4 @@ use core::fmt::{self, Debug, Formatter}; -#[cfg(windows)] -use std::process::abort; use std::{ffi::c_void, marker::PhantomData}; use frida_gum::{ @@ -112,7 +110,7 @@ where log::error!("Crashing target as it had ASan errors"); libc::raise(libc::SIGABRT); #[cfg(windows)] - abort(); + std::process::abort(); } } self.helper.post_exec(input)?; @@ -220,7 +218,7 @@ where } #[cfg(windows)] - initialize(&gum); + initialize(gum); Self { base, diff --git a/libafl_frida/src/lib.rs b/libafl_frida/src/lib.rs index 4edee14caa..f30cede48b 100644 --- a/libafl_frida/src/lib.rs +++ b/libafl_frida/src/lib.rs @@ -22,7 +22,8 @@ Additional documentation is available in [the `LibAFL` book](https://aflplus.plu clippy::module_name_repetitions, clippy::unreadable_literal, clippy::ptr_cast_constness, - clippy::must_use_candidate + clippy::must_use_candidate, + clippy::too_many_arguments )] #![cfg_attr(not(test), warn( missing_debug_implementations, @@ -70,7 +71,7 @@ pub mod alloc; pub mod asan; #[cfg(windows)] -/// Windows specific hooks to catch __fastfail like exceptions with Frida, see https://github.com/AFLplusplus/LibAFL/issues/395 for more details +/// Windows specific hooks to catch __fastfail like exceptions with Frida, see for more details pub mod windows_hooks; pub mod coverage_rt; diff --git a/libafl_frida/src/windows_hooks.rs b/libafl_frida/src/windows_hooks.rs index 73c04c9121..9ceb2b06b7 100644 --- a/libafl_frida/src/windows_hooks.rs +++ b/libafl_frida/src/windows_hooks.rs @@ -1,27 +1,42 @@ // Based on the example of setting hooks: Https://github.com/frida/frida-rust/blob/main/examples/gum/hook_open/src/lib.rs +use std::ffi::c_void; + use frida_gum::{interceptor::Interceptor, Gum, Module, NativePointer}; use libafl_bolts::os::windows_exceptions::{ handle_exception, IsProcessorFeaturePresent, UnhandledExceptionFilter, EXCEPTION_POINTERS, PROCESSOR_FEATURE_ID, }; +unsafe extern "C" fn is_processor_feature_present_detour(feature: u32) -> bool { + match feature { + 0x17 => false, + _ => IsProcessorFeaturePresent(PROCESSOR_FEATURE_ID(feature)).as_bool(), + } +} +unsafe extern "C" fn unhandled_exception_filter_detour( + exception_pointers: *mut EXCEPTION_POINTERS, +) -> i32 { + handle_exception(exception_pointers); + UnhandledExceptionFilter(exception_pointers) +} /// Initialize the hooks pub fn initialize(gum: &Gum) { let is_processor_feature_present = Module::find_export_by_name(Some("kernel32.dll"), "IsProcessorFeaturePresent"); let is_processor_feature_present = is_processor_feature_present.unwrap(); - if is_processor_feature_present.is_null() { - panic!("IsProcessorFeaturePresent not found"); - } + assert!( + !is_processor_feature_present.is_null(), + "IsProcessorFeaturePresent not found" + ); let unhandled_exception_filter = Module::find_export_by_name(Some("kernel32.dll"), "UnhandledExceptionFilter"); let unhandled_exception_filter = unhandled_exception_filter.unwrap(); - if unhandled_exception_filter.is_null() { - panic!("UnhandledExceptionFilter not found"); - } + assert!( + !unhandled_exception_filter.is_null(), + "UnhandledExceptionFilter not found" + ); - let mut interceptor = Interceptor::obtain(&gum); - use std::ffi::c_void; + let mut interceptor = Interceptor::obtain(gum); interceptor .replace( @@ -29,7 +44,7 @@ pub fn initialize(gum: &Gum) { NativePointer(is_processor_feature_present_detour as *mut c_void), NativePointer(std::ptr::null_mut()), ) - .unwrap_or_else(|_| NativePointer(std::ptr::null_mut())); + .unwrap_or(NativePointer(std::ptr::null_mut())); interceptor .replace( @@ -37,20 +52,5 @@ pub fn initialize(gum: &Gum) { NativePointer(unhandled_exception_filter_detour as *mut c_void), NativePointer(std::ptr::null_mut()), ) - .unwrap_or_else(|_| NativePointer(std::ptr::null_mut())); - - unsafe extern "C" fn is_processor_feature_present_detour(feature: u32) -> bool { - let result = match feature { - 0x17 => false, - _ => IsProcessorFeaturePresent(PROCESSOR_FEATURE_ID(feature)).as_bool(), - }; - result - } - - unsafe extern "C" fn unhandled_exception_filter_detour( - exception_pointers: *mut EXCEPTION_POINTERS, - ) -> i32 { - handle_exception(exception_pointers); - UnhandledExceptionFilter(exception_pointers) - } + .unwrap_or(NativePointer(std::ptr::null_mut())); } diff --git a/libafl_qemu/build_linux.rs b/libafl_qemu/build_linux.rs index 210cc4dee3..83b3420590 100644 --- a/libafl_qemu/build_linux.rs +++ b/libafl_qemu/build_linux.rs @@ -137,8 +137,8 @@ pub fn build() { maybe_generate_stub_bindings( &cpu_target, &emulation_mode, - &stub_runtime_bindings_file, - &runtime_bindings_file + stub_runtime_bindings_file.as_path(), + runtime_bindings_file.as_path() ); if (emulation_mode == "usermode") && (qemu_asan || qemu_asan_guest) { diff --git a/libafl_qemu/libafl_qemu_build/src/lib.rs b/libafl_qemu/libafl_qemu_build/src/lib.rs index a3b860a56c..631263a31b 100644 --- a/libafl_qemu/libafl_qemu_build/src/lib.rs +++ b/libafl_qemu/libafl_qemu_build/src/lib.rs @@ -252,7 +252,7 @@ fn include_path(build_dir: &Path, path: &str) -> String { /// If `fresh_content` != `content_file_to_update` (the file is read directly if `content_file_to_update` is None), update the file. prefix is not considered for comparison. /// If a prefix is given, it will be added as the first line of the file. pub fn store_generated_content_if_different( - file_to_update: &PathBuf, + file_to_update: &Path, fresh_content: &[u8], content_file_to_update: Option>, first_line_prefix: Option<&str>, @@ -310,8 +310,8 @@ pub fn store_generated_content_if_different( pub fn maybe_generate_stub_bindings( cpu_target: &str, emulation_mode: &str, - stub_bindings_file: &PathBuf, - bindings_file: &PathBuf, + stub_bindings_file: &Path, + bindings_file: &Path, ) { if cpu_target == "x86_64" && emulation_mode == "usermode" { let current_rustc_version = @@ -389,8 +389,8 @@ pub fn maybe_generate_stub_bindings( pub fn maybe_generate_stub_bindings( _cpu_target: &str, _emulation_mode: &str, - _stub_bindings_file: &PathBuf, - _bindings_file: &PathBuf, + _stub_bindings_file: &Path, + _bindings_file: &Path, ) { // Do nothing } diff --git a/libafl_qemu/libafl_qemu_sys/build_linux.rs b/libafl_qemu/libafl_qemu_sys/build_linux.rs index ab80aeafd2..5002b41c0b 100644 --- a/libafl_qemu/libafl_qemu_sys/build_linux.rs +++ b/libafl_qemu/libafl_qemu_sys/build_linux.rs @@ -100,7 +100,7 @@ pub fn build() { maybe_generate_stub_bindings( &cpu_target, &emulation_mode, - &stub_bindings_file, - &bindings_file, + stub_bindings_file.as_path(), + bindings_file.as_path(), ); } diff --git a/libafl_targets/src/windows_asan.rs b/libafl_targets/src/windows_asan.rs index 86bd9d7169..28244d9b89 100644 --- a/libafl_targets/src/windows_asan.rs +++ b/libafl_targets/src/windows_asan.rs @@ -12,7 +12,7 @@ use libafl::{ pub type CB = unsafe extern "C" fn() -> (); extern "C" { - fn __sanitizer_set_death_callback(cb: CB); + fn __sanitizer_set_death_callback(cb: Option); } /// Setup `ASan` callback on windows @@ -35,5 +35,5 @@ where E::State: HasSolutions + HasCorpus + HasExecutions, Z: HasObjective, { - __sanitizer_set_death_callback(asan_death_handler::); + __sanitizer_set_death_callback(Some(asan_death_handler::)); } diff --git a/scripts/clippy.ps1 b/scripts/clippy.ps1 index 33415c9cfc..76be0592a5 100644 --- a/scripts/clippy.ps1 +++ b/scripts/clippy.ps1 @@ -1,4 +1,4 @@ -cargo clippy --all --all-features --tests --benches --examples -- ` +cargo clippy --all --all-features --exclude libafl_nyx --exclude symcc_runtime --exclude runtime_test --exclude libafl_qemu --exclude libafl_libfuzzer --exclude libafl_qemu_sys --no-deps --tests --benches --examples -- ` -D clippy::all ` -D clippy::pedantic ` -W clippy::similar_names `