Fix Arbitrary External Call bug (#187)

* fix sourcemap issue

* add two oracles

* add returns to call printer

* bug fix

* on return removal

* bugfix

* support arb calls in vm

* revm bump

* bump revm

* add arbitrary_external_call oracle

cli/src/evm.rs

@@ -176,6 +176,9 @@ pub struct EvmArgs {
     #[arg(long, default_value = "true")]
     selfdestruct_oracle: bool,
 
+    #[arg(long, default_value = "true")]
+    arbitrary_external_call_oracle: bool,
+
     #[arg(long, default_value = "true")]
     echidna_oracle: bool,
 
@@ -561,6 +564,7 @@ pub fn evm_main(args: EvmArgs) {
         spec_id: args.spec_id,
         typed_bug: args.typed_bug_oracle,
         selfdestruct_bug: args.selfdestruct_oracle,
+        arbitrary_external_call: args.arbitrary_external_call_oracle,
         builder,
     };
 

src/evm/concolic/concolic_host.rs

@@ -1326,6 +1326,7 @@ where
         interp: &mut Interpreter,
         host: &mut FuzzHost<VS, I, S>,
         state: &mut S,
+        by: &Bytes
     ) {
         self.pop_ctx();
     }

src/evm/config.rs

@@ -74,5 +74,6 @@ pub struct Config<VS, Addr, Code, By, Loc, SlotTy, Out, I, S, CI> {
     pub only_fuzz: HashSet<EVMAddress>,
     pub typed_bug: bool,
     pub selfdestruct_bug: bool,
+    pub arbitrary_external_call: bool,
     pub builder: Option<BuildJob>,
 }

src/evm/host.rs

@@ -122,7 +122,7 @@ where
     pub _pc: usize,
     pub pc_to_addresses: HashMap<(EVMAddress, usize), HashSet<EVMAddress>>,
     pub pc_to_create: HashMap<(EVMAddress, usize), usize>,
-    pub pc_to_call_hash: HashMap<(EVMAddress, usize), HashSet<Vec<u8>>>,
+    pub pc_to_call_hash: HashMap<(EVMAddress, usize, usize), HashSet<Vec<u8>>>,
     pub middlewares_enabled: bool,
     pub middlewares: Rc<RefCell<Vec<Rc<RefCell<dyn Middleware<VS, I, S>>>>>>,
 
@@ -149,6 +149,8 @@ where
     pub setcode_data: HashMap<EVMAddress, Bytecode>,
     // selftdestruct
     pub current_self_destructs: Vec<(EVMAddress, usize)>,
+    // arbitrary calls
+    pub current_arbitrary_calls: Vec<(EVMAddress, EVMAddress, usize)>,
     // relations file handle
     relations_file: std::fs::File,
     // Filter duplicate relations
@@ -168,6 +170,8 @@ where
 
     /// For future continue executing when control leak happens
     pub leak_ctx: Vec<SinglePostExecution>,
+
+    pub jumpi_trace: usize,
 }
 
 impl<VS, I, S> Debug for FuzzHost<VS, I, S>
@@ -227,6 +231,7 @@ where
             logs: Default::default(),
             setcode_data: self.setcode_data.clone(),
             current_self_destructs: self.current_self_destructs.clone(),
+            current_arbitrary_calls: self.current_arbitrary_calls.clone(),
             relations_file: self.relations_file.try_clone().unwrap(),
             relations_hash: self.relations_hash.clone(),
             current_typed_bug: self.current_typed_bug.clone(),
@@ -237,14 +242,15 @@ where
             leak_ctx: self.leak_ctx.clone(),
             mapping_sstore_pcs: self.mapping_sstore_pcs.clone(),
             mapping_sstore_pcs_to_slot: self.mapping_sstore_pcs_to_slot.clone(),
+            jumpi_trace: self.jumpi_trace,
         }
     }
 }
 
 // hack: I don't want to change evm internal to add a new type of return
 // this return type is never used as we disabled gas
 pub static mut ACTIVE_MATCH_EXT_CALL: bool = false;
-const CONTROL_LEAK_DETECTION: bool = true;
+const CONTROL_LEAK_DETECTION: bool = false;
 const UNBOUND_CALL_THRESHOLD: usize = 3;
 
 // if a PC transfers control to >2 addresses, we consider call at this PC to be unbounded
@@ -289,6 +295,7 @@ where
             logs: Default::default(),
             setcode_data: HashMap::new(),
             current_self_destructs: Default::default(),
+            current_arbitrary_calls: Default::default(),
             relations_file: std::fs::File::create(format!("{}/relations.log", workdir)).unwrap(),
             relations_hash: HashSet::new(),
             current_typed_bug: Default::default(),
@@ -299,6 +306,7 @@ where
             leak_ctx: vec![],
             mapping_sstore_pcs: Default::default(),
             mapping_sstore_pcs_to_slot: Default::default(),
+            jumpi_trace: 37,
         };
         // ret.env.block.timestamp = EVMU256::max_value();
         ret
@@ -604,29 +612,33 @@ where
             // check whether the whole CALLDATAVALUE can be arbitrary
             if !self
                 .pc_to_call_hash
-                .contains_key(&(input.context.caller, self._pc))
+                .contains_key(&(input.context.caller, self._pc, self.jumpi_trace))
             {
                 self.pc_to_call_hash
-                    .insert((input.context.caller, self._pc), HashSet::new());
+                    .insert((input.context.caller, self._pc, self.jumpi_trace), HashSet::new());
             }
             self.pc_to_call_hash
-                .get_mut(&(input.context.caller, self._pc))
+                .get_mut(&(input.context.caller, self._pc, self.jumpi_trace))
                 .unwrap()
                 .insert(hash.to_vec());
             if self
                 .pc_to_call_hash
-                .get(&(input.context.caller, self._pc))
+                .get(&(input.context.caller, self._pc, self.jumpi_trace))
                 .unwrap()
                 .len()
                 > UNBOUND_CALL_THRESHOLD
                 && input_seq.len() >= 4
             {
-                // println!("ub leak {:?} -> {:?} with {:?}", input.context.caller, input.contract, hex::encode(input.input.clone()));
+                self.current_arbitrary_calls.push(
+                    (input.context.caller, input.context.address, interp.program_counter()),
+                );
+                // println!("ub leak {:?} -> {:?} with {:?} {}", input.context.caller, input.contract, hex::encode(input.input.clone()), self.jumpi_trace);
                 push_interp!();
                 return (
                     InstructionResult::ArbitraryExternalCallAddressBounded(
                         input.context.caller,
                         input.context.address,
+                        input.transfer.value
                     ),
                     Gas::new(0),
                     Bytes::new(),
@@ -690,7 +702,7 @@ where
         // if there is code, then call the code
         let res = self.call_forbid_control_leak(input, state);
         match res.0 {
-            ControlLeak | InstructionResult::ArbitraryExternalCallAddressBounded(_, _) => unsafe {
+            ControlLeak | InstructionResult::ArbitraryExternalCallAddressBounded(_, _, _) => unsafe {
                 unsafe {
                     self.leak_ctx.push(SinglePostExecution::from_interp(
                         interp,
@@ -873,7 +885,11 @@ where
                     } else {
                         as_u64(fast_peek!(0))
                     };
-                    let idx = (interp.program_counter() * (jump_dest as usize)) % MAP_SIZE;
+                    let _pc = interp.program_counter();
+
+                    let (shash, _) = self.jumpi_trace.overflowing_mul(54059);
+                    self.jumpi_trace = (shash) ^ (_pc * 76963);
+                    let idx = (_pc * (jump_dest as usize)) % MAP_SIZE;
                     if JMP_MAP[idx] == 0 {
                         self.coverage_changed = true;
                     }
@@ -1286,8 +1302,19 @@ where
             }
         };
 
+        let ret_buffer = res.2.clone();
+
         unsafe {
-            invoke_middlewares!(self, interp, state, on_return);
+            if self.middlewares_enabled {
+                for middleware in &mut self.middlewares.clone().deref().borrow_mut().iter_mut()
+                {
+                    middleware
+                        .deref()
+                        .deref()
+                        .borrow_mut()
+                        .on_return(interp, self, state, &ret_buffer);
+                }
+            }
         }
         res
     }

src/evm/middlewares/call_printer.rs

@@ -6,6 +6,7 @@ use std::io::Write;
 use std::ops::AddAssign;
 use std::path::Path;
 use std::time::{SystemTime, UNIX_EPOCH};
+use bytes::Bytes;
 use itertools::Itertools;
 use libafl::inputs::Input;
 use libafl::prelude::{HasCorpus, HasMetadata, State};
@@ -49,6 +50,7 @@ pub struct SingleCall {
     pub input: String,
     pub value: String,
     pub source: Option<SourceMapLocation>,
+    pub results: String
 }
 
 #[derive(Clone, Debug, Serialize, Default, Deserialize)]
@@ -62,6 +64,7 @@ pub struct CallPrinter {
     pub sourcemaps: ProjectSourceMapTy,
     pub current_layer: usize,
     pub results: CallPrinterResult,
+    pub offsets: usize,
 
     entry: bool
 }
@@ -77,6 +80,7 @@ impl CallPrinter {
             current_layer: 0,
             results: Default::default(),
             entry: true,
+            offsets: 0
         }
     }
 
@@ -99,7 +103,7 @@ impl CallPrinter {
     pub fn get_trace(&self) -> String {
         self.results.data.iter().map(|(layer, call)| {
             let padding = (0..*layer).map(|_| "  ").join("");
-            format!("{}[{} -> {}] ({})", padding, call.caller, call.contract, call.input)
+            format!("{}[{} -> {}] ({}) > ({})", padding, call.caller, call.contract, call.input, call.results)
         }).join("\n")
     }
 
@@ -173,6 +177,7 @@ impl<I, VS, S> Middleware<VS, I, S> for CallPrinter
                 } else {
                     None
                 },
+                results: "".to_string(),
             }));
         }
 
@@ -241,6 +246,7 @@ impl<I, VS, S> Middleware<VS, I, S> for CallPrinter
             },
         ).unwrap_or(vec![]);
 
+        self.offsets = 0;
         self.results.data.push((self.current_layer, SingleCall {
             call_type,
             caller: self.translate_address(caller),
@@ -259,6 +265,7 @@ impl<I, VS, S> Middleware<VS, I, S> for CallPrinter
             } else {
                 None
             },
+            results: "".to_string(),
         }));
     }
 
@@ -267,7 +274,13 @@ impl<I, VS, S> Middleware<VS, I, S> for CallPrinter
         interp: &mut Interpreter,
         host: &mut FuzzHost<VS, I, S>,
         state: &mut S,
+        by: &Bytes
     ) {
+        self.offsets += 1;
+        let l = self.results.data.len();
+        self.results.data[l - self.offsets]
+            .1.results = hex::encode(by);
+
         self.current_layer -= 1;
     }
 

src/evm/middlewares/coverage.rs

@@ -346,13 +346,6 @@ impl<I, VS, S> Middleware<VS, I, S> for Coverage
     fn get_type(&self) -> MiddlewareType {
         MiddlewareType::InstructionCoverage
     }
-
-    unsafe fn on_return(
-        &mut self,
-        interp: &mut Interpreter,
-        host: &mut FuzzHost<VS, I, S>,
-        state: &mut S,
-    ) {}
 }
 
 

src/evm/middlewares/middleware.rs

@@ -99,7 +99,8 @@ where
         interp: &mut Interpreter,
         host: &mut FuzzHost<VS, I, S>,
         state: &mut S,
-    );
+        ret: &Bytes
+    ) {}
 
     unsafe fn on_insert(&mut self,
                         bytecode: &mut Bytecode,

src/evm/middlewares/sha3_bypass.rs

@@ -5,6 +5,7 @@ use crate::evm::types::{as_u64, EVMAddress, EVMU256};
 use crate::generic_vm::vm_state::VMStateT;
 use crate::input::VMInputT;
 use crate::state::{HasCaller, HasCurrentInputIdx, HasItyState};
+use bytes::Bytes;
 use itertools::Itertools;
 use libafl::inputs::Input;
 use libafl::prelude::{HasCorpus, HasMetadata, State};
@@ -385,10 +386,8 @@ where
     }
 
     unsafe fn on_return(
-        &mut self,
-        interp: &mut Interpreter,
-        host: &mut FuzzHost<VS, I, S>,
-        state: &mut S,
+        &mut self, interp: &mut Interpreter, host: &mut FuzzHost<VS, I, S>, state: &mut S,
+        by: &Bytes
     ) {
         self.pop_ctx();
     }
@@ -465,13 +464,6 @@ where
         MiddlewareType::Sha3Bypass
     }
 
-    unsafe fn on_return(
-        &mut self,
-        interp: &mut Interpreter,
-        host: &mut FuzzHost<VS, I, S>,
-        state: &mut S,
-    ) {
-    }
 }
 
 mod tests {

src/evm/mutator.rs

@@ -17,7 +17,7 @@ use crate::evm::input::EVMInputTy::Borrow;
 use std::fmt::Debug;
 use revm_interpreter::Interpreter;
 use crate::evm::abi::ABIAddressToInstanceMap;
-use crate::evm::types::{convert_u256_to_h160, EVMAddress};
+use crate::evm::types::{convert_u256_to_h160, EVMAddress, EVMU256};
 use crate::evm::vm::{Constraint, EVMState, EVMStateT};
 
 use crate::state::HasItyState;
@@ -128,6 +128,9 @@ impl<'a, VS, Loc, Addr, SC, CI> FuzzMutator<'a, VS, Loc, Addr, SC, CI>
                 Constraint::Caller(caller) => {
                     input.set_caller_evm(caller);
                 }
+                Constraint::Value(value) => {
+                    input.set_txn_value(value);
+                }
                 Constraint::Contract(target) => {
                     let rand_int = state.rand_mut().next();
                     let always_none = state.rand_mut().next() % 30 == 0;
@@ -155,6 +158,7 @@ impl<'a, VS, Loc, Addr, SC, CI> FuzzMutator<'a, VS, Loc, Addr, SC, CI>
                         input.set_liquidation_percent(0);
                     }
                 }
+                _ => {}
             }
         }
     }
@@ -201,7 +205,7 @@ impl<'a, VS, Loc, Addr, I, S, SC, CI> Mutator<I, S> for FuzzMutator<'a, VS, Loc,
             // if the input is a step input (resume execution from a control leak)
             // we should not mutate the VM state, but only mutate the bytes
             if input.is_step() {
-                return match state.rand_mut().below(100) {
+                let res = match state.rand_mut().below(100) {
                     #[cfg(feature = "flashloan_v2")]
                     0..=5 => {
                         let prev_percent = input.get_liquidation_percent();
@@ -218,6 +222,9 @@ impl<'a, VS, Loc, Addr, I, S, SC, CI> Mutator<I, S> for FuzzMutator<'a, VS, Loc,
                     }
                     _ => input.mutate(state),
                 };
+
+                input.set_txn_value(EVMU256::ZERO);
+                return res;
             }
 
             // if the input is to borrow token, we should mutate the randomness

src/evm/onchain/flashloan.rs

@@ -43,7 +43,6 @@ use crate::evm::types::convert_u256_to_h160;
 use crate::evm::types::float_scale_to_u512;
 use crate::evm::vm::IS_FAST_CALL_STATIC;
 
-const UNBOUND_TRANSFER_AMT: usize = 5;
 macro_rules! scale {
     () => {
         EVMU512::from(1_000_000)
@@ -503,13 +502,6 @@ where
 
     }
 
-    unsafe fn on_return(
-        &mut self,
-        interp: &mut Interpreter,
-        host: &mut FuzzHost<VS, I, S>,
-        state: &mut S,
-    ) {}
-
     fn get_type(&self) -> MiddlewareType {
         return MiddlewareType::Flashloan;
     }