use calling convention hints whenever possible (#420)

CHANGES.md

@@ -1,3 +1,6 @@
+0.8-dev
+===
+
 0.7 (2023-06)
 ====
 

src/caller/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cwe_checker"
-version = "0.7.0"
+version = "0.8.0-dev"
 authors = ["Nils-Edvin Enkelmann <nils-edvin.enkelmann@fkie.fraunhofer.de>"]
 edition = "2021"
 

src/cwe_checker_lib/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cwe_checker_lib"
-version = "0.7.0"
+version = "0.8.0-dev"
 authors = ["Nils-Edvin Enkelmann <nils-edvin.enkelmann@fkie.fraunhofer.de>"]
 edition = "2021"
 

src/cwe_checker_lib/src/analysis/function_signature/context.rs

@@ -450,12 +450,15 @@ impl<'a> forward_interprocedural_fixpoint::Context<'a> for Context<'a> {
         state_before_call: Option<&State>,
         call_term: &Term<Jmp>,
         _return_term: &Term<Jmp>,
-        _calling_convention: &Option<String>,
+        calling_convention_opt: &Option<String>,
     ) -> Option<State> {
         if state.is_none() || state_before_call.is_none() {
             return None;
         }
-        let calling_convention = match self.project.get_standard_calling_convention() {
+        let calling_convention = match self
+            .project
+            .get_specific_calling_convention(calling_convention_opt)
+        {
             Some(cconv) => cconv,
             None => return None,
         };

src/cwe_checker_lib/src/analysis/function_signature/mod.rs

@@ -65,11 +65,7 @@ fn generate_fixpoint_computation<'a>(
                     // The node of a function entry point
                     let calling_convention = project
                         .get_specific_calling_convention(&sub.term.calling_convention)
-                        .unwrap_or_else(|| {
-                            project
-                                .get_standard_calling_convention()
-                                .expect("No standard calling convention found.")
-                        });
+                        .expect("No standard calling convention found.");
                     let mut fn_start_state = State::new(
                         &sub.tid,
                         &project.stack_pointer_register,

src/cwe_checker_lib/src/analysis/graph.rs

@@ -477,8 +477,7 @@ impl<'a> GraphBuilder<'a> {
 
     /// Add all non-return-instruction-related jump edges to the graph.
     fn add_jump_and_call_edges(&mut self) {
-        while !self.block_worklist.is_empty() {
-            let node = self.block_worklist.pop().unwrap();
+        while let Some(node) = self.block_worklist.pop() {
             match self.graph[node] {
                 Node::BlkEnd(block, _) => self.add_outgoing_edges(node, block),
                 _ => panic!(),

src/cwe_checker_lib/src/checkers/cwe_476/context.rs

@@ -215,14 +215,26 @@ impl<'a> Context<'a> {
     /// If a possible  parameter register of the call contains taint,
     /// generate a CWE warning and return `None`.
     /// Else remove all taint contained in non-callee-saved registers.
-    fn handle_generic_call(&self, state: &State, call_tid: &Tid) -> Option<State> {
+    fn handle_generic_call(
+        &self,
+        state: &State,
+        call_tid: &Tid,
+        calling_convention_hint: &Option<String>,
+    ) -> Option<State> {
         let pi_state_option = self.get_current_pointer_inference_state(state, call_tid);
-        if state.check_generic_function_params_for_taint(self.project, pi_state_option.as_ref()) {
+        if state.check_generic_function_params_for_taint(
+            self.project,
+            pi_state_option.as_ref(),
+            calling_convention_hint,
+        ) {
             self.generate_cwe_warning(call_tid);
             return None;
         }
         let mut new_state = state.clone();
-        if let Some(calling_conv) = self.project.get_standard_calling_convention() {
+        if let Some(calling_conv) = self
+            .project
+            .get_specific_calling_convention(calling_convention_hint)
+        {
             new_state.remove_non_callee_saved_taint(calling_conv);
         }
         Some(new_state)
@@ -260,10 +272,14 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont
         state: &State,
         call: &Term<Jmp>,
         _target: &Node,
-        _calling_convention: &Option<String>,
+        calling_convention: &Option<String>,
     ) -> Option<Self::Value> {
         let pi_state_option = self.get_current_pointer_inference_state(state, &call.tid);
-        if state.check_generic_function_params_for_taint(self.project, pi_state_option.as_ref()) {
+        if state.check_generic_function_params_for_taint(
+            self.project,
+            pi_state_option.as_ref(),
+            calling_convention,
+        ) {
             self.generate_cwe_warning(&call.tid);
         }
         None
@@ -295,7 +311,7 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont
                     panic!("Extern symbol not found.");
                 }
             }
-            Jmp::CallInd { .. } => self.handle_generic_call(state, &call.tid),
+            Jmp::CallInd { .. } => self.handle_generic_call(state, &call.tid, &None),
             _ => panic!("Malformed control flow graph encountered."),
         }
     }
@@ -391,18 +407,22 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont
         state_before_call: Option<&State>,
         call_term: &Term<Jmp>,
         return_term: &Term<Jmp>,
-        _calling_convention: &Option<String>,
+        calling_convention: &Option<String>,
     ) -> Option<State> {
         if let Some(state) = state_before_return {
             // If taint is returned, generate a CWE warning
             let pi_state_option = self.get_current_pointer_inference_state(state, &return_term.tid);
-            if state.check_return_values_for_taint(self.project, pi_state_option.as_ref()) {
+            if state.check_return_values_for_taint(
+                self.project,
+                pi_state_option.as_ref(),
+                calling_convention,
+            ) {
                 self.generate_cwe_warning(&return_term.tid);
             }
             // Do not return early in case `state_before_call` is also set (possible for recursive functions).
         }
         if let Some(state) = state_before_call {
-            self.handle_generic_call(state, &call_term.tid)
+            self.handle_generic_call(state, &call_term.tid, calling_convention)
         } else {
             None
         }
@@ -471,12 +491,12 @@ mod tests {
         let mut state = State::mock();
 
         assert!(context
-            .handle_generic_call(&state, &Tid::new("call_tid"))
+            .handle_generic_call(&state, &Tid::new("call_tid"), &None)
             .is_some());
 
         state.set_register_taint(&variable!("RDX:8"), Taint::Tainted(ByteSize::new(8)));
         assert!(context
-            .handle_generic_call(&state, &Tid::new("call_tid"))
+            .handle_generic_call(&state, &Tid::new("call_tid"), &None)
             .is_none());
     }
 

src/cwe_checker_lib/src/checkers/cwe_476/state.rs

@@ -275,14 +275,17 @@ impl State {
     }
 
     /// Check whether a generic function call may contain tainted values in its parameters.
-    /// Since we don't know the actual calling convention of the call,
-    /// we approximate the parameters with all parameter registers of the standard calling convention of the project.
+    /// Since we don't know the actual parameters of the call,
+    /// we approximate the parameters with all parameter registers of the calling convention of the function
+    /// or of the standard calling convention of the project.
     pub fn check_generic_function_params_for_taint(
         &self,
         project: &Project,
         pi_state_option: Option<&PointerInferenceState>,
+        calling_convention_hint: &Option<String>,
     ) -> bool {
-        if let Some(calling_conv) = project.get_standard_calling_convention() {
+        if let Some(calling_conv) = project.get_specific_calling_convention(calling_convention_hint)
+        {
             let mut all_parameters = calling_conv.integer_parameter_register.clone();
             for float_param in calling_conv.float_parameter_register.iter() {
                 for var in float_param.input_vars() {
@@ -298,13 +301,16 @@ impl State {
 
     /// Check whether the return registers may contain tainted values or point to objects containing tainted values.
     /// Since we don't know the actual return registers,
-    /// we approximate them by all return registers of the standard calling convention of the project.
+    /// we approximate them by all return registers of the calling convention of the function
+    /// or of the standard calling convention of the project.
     pub fn check_return_values_for_taint(
         &self,
         project: &Project,
         pi_state_option: Option<&PointerInferenceState>,
+        calling_convention_hint: &Option<String>,
     ) -> bool {
-        if let Some(calling_conv) = project.get_standard_calling_convention() {
+        if let Some(calling_conv) = project.get_specific_calling_convention(calling_convention_hint)
+        {
             self.check_register_list_for_taint(
                 &calling_conv.integer_return_register[..],
                 pi_state_option,

src/cwe_checker_lib/src/intermediate_representation/project.rs

@@ -47,16 +47,20 @@ impl Project {
     }
 
     /// Try to find a specific calling convention in the list of calling conventions in the project.
-    /// If not given a calling convention (i.e. given `None`) then falls back to `get_standard_calling_convention`
+    /// If not given a calling convention (i.e. given `None`) or the given calling convention name was not found
+    /// then falls back to `get_standard_calling_convention`.
     pub fn get_specific_calling_convention(
         &self,
         cconv_name_opt: &Option<String>,
     ) -> Option<&CallingConvention> {
-        if let Some(cconv_name) = cconv_name_opt {
-            self.calling_conventions.get(cconv_name)
-        } else {
-            self.get_standard_calling_convention()
-        }
+        // FIXME: On x86 Windows binaries we can get a strange edge case:
+        // For some reason we get cases where Ghidra annotates a function with `__cdecl` as calling convention,
+        // but the general calling convention list only contains `__fastcall` and `__thiscall`.
+        // We should investigate this, so that we do not have to fall back to the standard calling convention.
+        cconv_name_opt
+            .as_ref()
+            .and_then(|cconv_name| self.calling_conventions.get(cconv_name))
+            .or_else(|| self.get_standard_calling_convention())
     }
 
     /// Return the calling convention associated to the given extern symbol.