From aa3f12610007ca7c4ce73cec09a48987da80df2b Mon Sep 17 00:00:00 2001 From: Langston Barrett Date: Sun, 9 Apr 2023 15:27:27 -0400 Subject: [PATCH] LibAFL_qemu: Return errors from `Emulator::new` instead of asserting (#1197) * qemu: Return errors from Emulator::new instead of asserting Libraries should not `assert!` except in cases of unrecoverable (library) programmer error. These errors are all potentially recoverable, and aren't internal errors in `libafl_qemu` itself. * Respond to review comments --- fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs | 2 +- fuzzers/fuzzbench_qemu/src/fuzzer.rs | 2 +- fuzzers/qemu_arm_launcher/src/fuzzer.rs | 2 +- fuzzers/qemu_launcher/src/fuzzer.rs | 2 +- fuzzers/qemu_systemmode/src/fuzzer.rs | 2 +- libafl/src/bolts/cli.rs | 2 +- libafl_qemu/src/asan.rs | 8 ++- libafl_qemu/src/emu.rs | 69 ++++++++++++++++++----- 8 files changed, 66 insertions(+), 23 deletions(-) diff --git a/fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs b/fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs index 9999fa272f..6baac70420 100644 --- a/fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs +++ b/fuzzers/fuzzbench_fork_qemu/src/fuzzer.rs @@ -147,7 +147,7 @@ fn fuzz( let args: Vec = env::args().collect(); let env: Vec<(String, String)> = env::vars().collect(); - let emu = Emulator::new(&args, &env); + let emu = Emulator::new(&args, &env)?; let mut elf_buffer = Vec::new(); let elf = EasyElf::from_file(emu.binary_path(), &mut elf_buffer)?; diff --git a/fuzzers/fuzzbench_qemu/src/fuzzer.rs b/fuzzers/fuzzbench_qemu/src/fuzzer.rs index 9486834254..e872e38281 100644 --- a/fuzzers/fuzzbench_qemu/src/fuzzer.rs +++ b/fuzzers/fuzzbench_qemu/src/fuzzer.rs @@ -171,7 +171,7 @@ fn fuzz( let args: Vec = env::args().collect(); let env: Vec<(String, String)> = env::vars().collect(); - let emu = Emulator::new(&args, &env); + let emu = Emulator::new(&args, &env).unwrap(); //let emu = init_with_asan(&mut args, &mut env); let mut elf_buffer = Vec::new(); diff --git a/fuzzers/qemu_arm_launcher/src/fuzzer.rs b/fuzzers/qemu_arm_launcher/src/fuzzer.rs index 215ca354aa..57cbc13b08 100644 --- a/fuzzers/qemu_arm_launcher/src/fuzzer.rs +++ b/fuzzers/qemu_arm_launcher/src/fuzzer.rs @@ -54,7 +54,7 @@ pub fn fuzz() { env::remove_var("LD_LIBRARY_PATH"); let args: Vec = env::args().collect(); let env: Vec<(String, String)> = env::vars().collect(); - let emu = Emulator::new(&args, &env); + let emu = Emulator::new(&args, &env).unwrap(); let mut elf_buffer = Vec::new(); let elf = EasyElf::from_file(emu.binary_path(), &mut elf_buffer).unwrap(); diff --git a/fuzzers/qemu_launcher/src/fuzzer.rs b/fuzzers/qemu_launcher/src/fuzzer.rs index 7bd59fb616..a38fad3c61 100644 --- a/fuzzers/qemu_launcher/src/fuzzer.rs +++ b/fuzzers/qemu_launcher/src/fuzzer.rs @@ -54,7 +54,7 @@ pub fn fuzz() { env::remove_var("LD_LIBRARY_PATH"); let args: Vec = env::args().collect(); let env: Vec<(String, String)> = env::vars().collect(); - let emu = Emulator::new(&args, &env); + let emu = Emulator::new(&args, &env).unwrap(); let mut elf_buffer = Vec::new(); let elf = EasyElf::from_file(emu.binary_path(), &mut elf_buffer).unwrap(); diff --git a/fuzzers/qemu_systemmode/src/fuzzer.rs b/fuzzers/qemu_systemmode/src/fuzzer.rs index 814c817d47..74fbd0b1dd 100644 --- a/fuzzers/qemu_systemmode/src/fuzzer.rs +++ b/fuzzers/qemu_systemmode/src/fuzzer.rs @@ -80,7 +80,7 @@ pub fn fuzz() { // Initialize QEMU let args: Vec = env::args().collect(); let env: Vec<(String, String)> = env::vars().collect(); - let emu = Emulator::new(&args, &env); + let emu = Emulator::new(&args, &env).unwrap(); emu.set_breakpoint(main_addr); unsafe { diff --git a/libafl/src/bolts/cli.rs b/libafl/src/bolts/cli.rs index 180eea5dfa..fe9275ba70 100644 --- a/libafl/src/bolts/cli.rs +++ b/libafl/src/bolts/cli.rs @@ -41,7 +41,7 @@ //! //! let env: Vec<(String, String)> = env::vars().collect(); //! -//! let emu = Emulator::new(&mut options.qemu_args.to_vec(), &mut env); +//! let emu = Emulator::new(&mut options.qemu_args.to_vec(), &mut env).unwrap(); //! // do other stuff... //! } //! diff --git a/libafl_qemu/src/asan.rs b/libafl_qemu/src/asan.rs index 5f2ca8305f..427a17a2b1 100644 --- a/libafl_qemu/src/asan.rs +++ b/libafl_qemu/src/asan.rs @@ -16,7 +16,7 @@ use meminterval::{Interval, IntervalTree}; use num_enum::{IntoPrimitive, TryFromPrimitive}; use crate::{ - emu::{Emulator, MemAccessInfo, SyscallHookResult}, + emu::{EmuError, Emulator, MemAccessInfo, SyscallHookResult}, helper::{QemuHelper, QemuHelperTuple, QemuInstrumentationFilter}, hooks::QemuHooks, GuestAddr, @@ -454,8 +454,10 @@ impl AsanGiovese { static mut ASAN_INITED: bool = false; -pub fn init_with_asan(args: &mut Vec, env: &mut [(String, String)]) -> Emulator { - assert!(!args.is_empty()); +pub fn init_with_asan( + args: &mut Vec, + env: &mut [(String, String)], +) -> Result { let current = env::current_exe().unwrap(); let asan_lib = fs::canonicalize(current) .unwrap() diff --git a/libafl_qemu/src/emu.rs b/libafl_qemu/src/emu.rs index f3cc4e29a4..c9f0ccb6cf 100644 --- a/libafl_qemu/src/emu.rs +++ b/libafl_qemu/src/emu.rs @@ -3,6 +3,7 @@ use core::{ convert::Into, ffi::c_void, + fmt, mem::MaybeUninit, ptr::{addr_of, copy_nonoverlapping, null}, }; @@ -673,28 +674,68 @@ pub struct Emulator { _private: (), } +#[derive(Debug)] +pub enum EmuError { + MultipleInstances, + EmptyArgs, + TooManyArgs(usize), +} + +impl std::error::Error for EmuError {} + +impl fmt::Display for EmuError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + EmuError::MultipleInstances => { + write!(f, "Only one instance of the QEMU Emulator is permitted") + } + EmuError::EmptyArgs => { + write!(f, "QEMU emulator args cannot be empty") + } + EmuError::TooManyArgs(n) => { + write!( + f, + "Too many arguments passed to QEMU emulator ({n} > i32::MAX)" + ) + } + } + } +} + +impl From for libafl::Error { + fn from(err: EmuError) -> Self { + libafl::Error::unknown(format!("{err}")) + } +} + #[allow(clippy::unused_self)] impl Emulator { #[allow(clippy::must_use_candidate, clippy::similar_names)] - pub fn new(args: &[String], env: &[(String, String)]) -> Emulator { + pub fn new(args: &[String], env: &[(String, String)]) -> Result { unsafe { - assert!( - !EMULATOR_IS_INITIALIZED, - "Only an instance of Emulator is permitted" - ); + if EMULATOR_IS_INITIALIZED { + return Err(EmuError::MultipleInstances); + } } - assert!(!args.is_empty()); + if args.is_empty() { + return Err(EmuError::EmptyArgs); + } + + let argc = args.len(); + if i32::try_from(argc).is_err() { + return Err(EmuError::TooManyArgs(argc)); + } + #[allow(clippy::cast_possible_wrap)] + let argc = argc as i32; + let args: Vec = args.iter().map(|x| x.clone() + "\0").collect(); let argv: Vec<*const u8> = args.iter().map(|x| x.as_bytes().as_ptr()).collect(); - assert!(argv.len() < i32::MAX as usize); let env_strs: Vec = env .iter() .map(|(k, v)| format!("{}={}\0", &k, &v)) .collect(); let mut envp: Vec<*const u8> = env_strs.iter().map(|x| x.as_bytes().as_ptr()).collect(); envp.push(null()); - #[allow(clippy::cast_possible_wrap)] - let argc = argv.len() as i32; unsafe { #[cfg(emulation_mode = "usermode")] qemu_user_init( @@ -714,7 +755,7 @@ impl Emulator { } EMULATOR_IS_INITIALIZED = true; } - Emulator { _private: () } + Ok(Emulator { _private: () }) } #[must_use] @@ -1174,10 +1215,10 @@ pub mod pybind { impl Emulator { #[allow(clippy::needless_pass_by_value)] #[new] - fn new(args: Vec, env: Vec<(String, String)>) -> Emulator { - Emulator { - emu: super::Emulator::new(&args, &env), - } + fn new(args: Vec, env: Vec<(String, String)>) -> PyResult { + let emu = super::Emulator::new(&args, &env) + .map_err(|e| PyValueError::new_err(format!("{e}")))?; + Ok(Emulator { emu }) } fn write_mem(&self, addr: GuestAddr, buf: &[u8]) {