Generic indirect call behaviour (#107)

cwe_checker_rs/src/analysis/fixpoint.rs

@@ -217,6 +217,11 @@ impl<T: Context> Computation<T> {
     pub fn get_context(&self) -> &T {
         &self.fp_context
     }
+
+    /// Returns `True` if the computation has stabilized, i.e. the internal worklist is empty.
+    pub fn has_stabilized(&self) -> bool {
+        self.worklist.is_empty()
+    }
 }
 
 #[cfg(test)]

cwe_checker_rs/src/analysis/graph.rs

@@ -30,6 +30,8 @@
 //! * Calls to library functions ([`image`](../../../../../doc/images/extern_calls.png)) outside the program are converted to *ExternCallStub* edges
 //! from the *BlkEnd* node of the callsite to the *BlkStart* node of the basic block the call returns to
 //! (if the call returns at all).
+//! * Right now indirect calls are handled as if they were extern calls, i.e. an *ExternCallStub* edge is added.
+//! This behaviour will change in the future, when better indirect call handling is implemented.
 //! * For each in-program call ([`image`](../../../../../doc/images/internal_function_call.png)) and corresponding return jump one node and three edges are generated:
 //!   * An artificial node *CallReturn*
 //!   * A *CRCallStub* edge from the *BlkEnd* node of the callsite to *CallReturn*
@@ -251,11 +253,22 @@ impl<'a> GraphBuilder<'a> {
                     }
                 }
             }
-            Jmp::CallInd {
-                target: _,
-                return_: _,
-            } => {
-                // TODO: add handling of indirect calls!
+            Jmp::CallInd { target: _, return_ } => {
+                // Right now we only add an artificial extern call stub for indirect calls.
+                // TODO: Handle cases where the call target may be known.
+                if let Some(return_tid) = return_ {
+                    let return_to_node = if let Some((return_to_node, _)) = self
+                        .jump_targets
+                        .get(&(return_tid.clone(), sub_term.tid.clone()))
+                    {
+                        *return_to_node
+                    } else {
+                        let return_block = self.program.term.find_block(return_tid).unwrap();
+                        self.add_block(return_block, sub_term).0
+                    };
+                    self.graph
+                        .add_edge(source, return_to_node, Edge::ExternCallStub(jump));
+                }
             }
             Jmp::CallOther {
                 description: _,

cwe_checker_rs/src/analysis/interprocedural_fixpoint.rs

@@ -81,7 +81,7 @@ pub trait Context<'a> {
     /// This way one can recover caller-specific information on return from a function.
     fn update_return(
         &self,
-        value: &Self::Value,
+        value: Option<&Self::Value>,
         value_before_call: Option<&Self::Value>,
         call_term: &Term<Jmp>,
         return_term: &Term<Jmp>,
@@ -192,18 +192,14 @@ impl<'a, T: Context<'a>> GeneralFPContext for GeneralizedContext<'a, T> {
                         _ => panic!("Malformed Control flow graph"),
                     };
                     let return_from_jmp = &return_from_block.term.jmps[0];
-                    if let Some(return_value) = return_ {
-                        match self.context.update_return(
-                            return_value,
-                            call.as_ref(),
-                            call_term,
-                            return_from_jmp,
-                        ) {
-                            Some(val) => Some(NodeValue::Value(val)),
-                            None => None,
-                        }
-                    } else {
-                        None
+                    match self.context.update_return(
+                        return_.as_ref(),
+                        call.as_ref(),
+                        call_term,
+                        return_from_jmp,
+                    ) {
+                        Some(val) => Some(NodeValue::Value(val)),
+                        None => None,
                     }
                 }
             },
@@ -282,6 +278,11 @@ impl<'a, T: Context<'a>> Computation<'a, T> {
     pub fn get_context(&self) -> &T {
         &self.generalized_computation.get_context().context
     }
+
+    /// Returns `True` if the computation has stabilized, i.e. the internal worklist is empty.
+    pub fn has_stabilized(&self) -> bool {
+        self.generalized_computation.has_stabilized()
+    }
 }
 
 /// Helper function to merge to values wrapped in `Option<..>`.

cwe_checker_rs/src/analysis/pointer_inference/context/mod.rs

@@ -258,6 +258,46 @@ impl<'a> Context<'a> {
         }
     }
 
+    /// Check whether the jump is an indirect call whose target evaluates to a *Top* value in the given state.
+    fn is_indirect_call_with_top_target(&self, state: &State, call: &Term<Jmp>) -> bool {
+        match &call.term {
+            Jmp::CallInd { target, .. }
+                if state.eval(target).map_or(false, |value| value.is_top()) =>
+            {
+                true
+            }
+            _ => false,
+        }
+    }
+
+    /// Adjust the stack register after a call to an extern function.
+    ///
+    /// On x86, this removes the return address from the stack
+    /// (other architectures pass the return address in a register, not on the stack).
+    /// On other architectures the stack register retains the value it had before the call.
+    /// Note that in some calling conventions the callee also clears function parameters from the stack.
+    /// We do not detect and handle these cases yet.
+    fn adjust_stack_register_on_extern_call(
+        &self,
+        state_before_call: &State,
+        new_state: &mut State,
+    ) {
+        let stack_register = &self.project.stack_pointer_register;
+        let stack_pointer = state_before_call.get_register(stack_register).unwrap();
+        match self.project.cpu_architecture.as_str() {
+            "x86" | "x86_64" => {
+                let offset = Bitvector::from_u64(stack_register.size.into())
+                    .into_truncate(apint::BitWidth::from(stack_register.size))
+                    .unwrap();
+                new_state.set_register(
+                    stack_register,
+                    stack_pointer.bin_op(BinOpType::IntAdd, &offset.into()),
+                );
+            }
+            _ => new_state.set_register(stack_register, stack_pointer),
+        }
+    }
+
     /// Handle an extern symbol call, whose concrete effect on the state is unknown.
     /// Basically, we assume that the call may write to all memory objects and register that is has access to.
     fn handle_generic_extern_call(
@@ -302,6 +342,46 @@ impl<'a> Context<'a> {
         Some(new_state)
     }
 
+    /// Handle a generic call whose target function is unknown.
+    ///
+    /// This function just assumes that the target of the call uses a reasonable standard calling convention
+    /// and that it may access (and write to) all parameter registers of this calling convention.
+    /// We also assume that the function does not use any parameters saved on the stack,
+    /// which may greatly reduce correctness of the analysis for the x86_32 architecture.
+    fn handle_call_to_generic_unknown_function(&self, state_before_call: &State) -> Option<State> {
+        if let Some(calling_conv) = self
+            .project
+            .calling_conventions
+            .iter()
+            .find(|cconv| cconv.name == "__stdcall")
+        {
+            let mut new_state = state_before_call.clone();
+            new_state.clear_non_callee_saved_register(&calling_conv.callee_saved_register[..]);
+            // Adjust stack register value (for x86 architecture).
+            self.adjust_stack_register_on_extern_call(state_before_call, &mut new_state);
+
+            let mut possible_referenced_ids = BTreeSet::new();
+            for parameter_register_name in calling_conv.parameter_register.iter() {
+                if let Some(register_value) =
+                    state_before_call.get_register_by_name(parameter_register_name)
+                {
+                    possible_referenced_ids.append(&mut register_value.referenced_ids());
+                }
+            }
+            possible_referenced_ids =
+                state_before_call.add_recursively_referenced_ids_to_id_set(possible_referenced_ids);
+            // Delete content of all referenced objects, as the function may write to them.
+            for id in possible_referenced_ids.iter() {
+                new_state
+                    .memory
+                    .assume_arbitrary_writes_to_object(id, &possible_referenced_ids);
+            }
+            Some(new_state)
+        } else {
+            None // We don't try to handle cases where we cannot guess a reasonable standard calling convention.
+        }
+    }
+
     /// Get the offset of the current stack pointer to the base of the current stack frame.
     fn get_current_stack_offset(&self, state: &State) -> BitvectorDomain {
         if let Ok(Data::Pointer(ref stack_pointer)) =

cwe_checker_rs/src/analysis/pointer_inference/context/tests.rs

@@ -172,7 +172,7 @@ fn context_problem_implementation() {
         .unwrap();
     let return_state = context
         .update_return(
-            &callee_state,
+            Some(&callee_state),
             Some(&state),
             &call,
             &return_term("return_target"),
@@ -340,7 +340,7 @@ fn update_return() {
 
     let state = context
         .update_return(
-            &state_before_return,
+            Some(&state_before_return),
             Some(&state_before_call),
             &call_term("callee"),
             &return_term("return_target"),

cwe_checker_rs/src/analysis/pointer_inference/context/trait_impls.rs

@@ -139,7 +139,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Context<'a> for Context<'a>
     /// The `state_before_call` is used to reconstruct caller-specific information like the caller stack frame.
     fn update_return(
         &self,
-        state_before_return: &State,
+        state_before_return: Option<&State>,
         state_before_call: Option<&State>,
         call_term: &Term<Jmp>,
         return_term: &Term<Jmp>,
@@ -149,11 +149,24 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Context<'a> for Context<'a>
         // When indirect calls are handled, the callsite alone is not a unique identifier anymore.
         // This may lead to confusion if both caller and callee have the same ID in their respective caller_stack_id sets.
 
-        // we only return to functions with a value before the call to prevent returning to dead code
-        let state_before_call = match state_before_call {
-            Some(value) => value,
-            None => return None,
-        };
+        let (state_before_call, state_before_return) =
+            match (state_before_call, state_before_return) {
+                (Some(state_call), Some(state_return)) => (state_call, state_return),
+                (Some(state_call), None) => {
+                    if self.is_indirect_call_with_top_target(state_call, call_term) {
+                        // We know nothing about the call target.
+                        return self.handle_call_to_generic_unknown_function(&state_call);
+                    } else {
+                        // We know at least something about the call target.
+                        // Since we don't have a return value,
+                        // we assume that the called function may not return at all.
+                        return None;
+                    }
+                }
+                (None, Some(_state_return)) => return None, // we only return to functions with a value before the call to prevent returning to dead code
+                (None, None) => return None,
+            };
+
         let original_caller_stack_id = &state_before_call.stack_id;
         let caller_stack_id = AbstractIdentifier::new(
             call_term.tid.clone(),
@@ -201,35 +214,28 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Context<'a> for Context<'a>
         Some(state_after_return)
     }
 
-    /// Update the state according to the effect of a call to an extern symbol.
+    /// Update the state according to the effect of a call to an extern symbol
+    /// or an indirect call where nothing is known about the call target.
     fn update_call_stub(&self, state: &State, call: &Term<Jmp>) -> Option<State> {
-        let mut new_state = state.clone();
         let call_target = match &call.term {
             Jmp::Call { target, .. } => target,
-            Jmp::CallInd { .. } => panic!("Indirect calls to extern symbols not yet supported."),
+            Jmp::CallInd { .. } => {
+                if self.is_indirect_call_with_top_target(state, call) {
+                    // We know nothing about the call target.
+                    return self.handle_call_to_generic_unknown_function(&state);
+                } else {
+                    return None;
+                }
+            }
             _ => panic!("Malformed control flow graph encountered."),
         };
+        let mut new_state = state.clone();
         if let Some(extern_symbol) = self.extern_symbol_map.get(call_target) {
             // Clear non-callee-saved registers from the state.
             let cconv = extern_symbol.get_calling_convention(&self.project);
             new_state.clear_non_callee_saved_register(&cconv.callee_saved_register[..]);
-            // On x86, remove the return address from the stack (other architectures pass the return address in a register, not on the stack).
-            // Note that in some calling conventions the callee also clears function parameters from the stack.
-            // We do not detect and handle these cases yet.
-            let stack_register = &self.project.stack_pointer_register;
-            let stack_pointer = state.get_register(stack_register).unwrap();
-            match self.project.cpu_architecture.as_str() {
-                "x86" | "x86_64" => {
-                    let offset = Bitvector::from_u64(stack_register.size.into())
-                        .into_truncate(apint::BitWidth::from(stack_register.size))
-                        .unwrap();
-                    new_state.set_register(
-                        stack_register,
-                        stack_pointer.bin_op(BinOpType::IntAdd, &offset.into()),
-                    );
-                }
-                _ => new_state.set_register(stack_register, stack_pointer),
-            }
+            // Adjust stack register value (for x86 architecture).
+            self.adjust_stack_register_on_extern_call(state, &mut new_state);
             // Check parameter for possible use-after-frees
             self.check_parameter_register_for_dangling_pointer(state, call, extern_symbol);