Minor project struct refactoring (#231)

src/caller/src/main.rs

@@ -164,8 +164,8 @@ fn run_with_ghidra(args: &CmdlineArgs) {
         .program
         .term
         .extern_symbols
-        .iter()
-        .map(|symbol| symbol.tid.clone())
+        .keys()
+        .cloned()
         .collect();
     let control_flow_graph = graph::get_program_cfg(&project.program, extern_sub_tids);
 

src/cwe_checker_lib/src/analysis/backward_interprocedural_fixpoint/tests.rs

-use crate::intermediate_representation::*;
-
 use super::{create_computation, mock_context, NodeValue};
+use crate::intermediate_representation::*;
+use std::collections::BTreeMap;
 
 use mock_context::Context;
 use mock_context::StartEnd;
@@ -128,7 +128,7 @@ fn mock_program() -> Term<Program> {
         tid: Tid::new("program"),
         term: Program {
             subs: vec![sub1, sub2],
-            extern_symbols: Vec::new(),
+            extern_symbols: BTreeMap::new(),
             entry_points: Vec::new(),
             address_base_offset: 0,
         },

src/cwe_checker_lib/src/analysis/dead_variable_elimination/mod.rs

@@ -18,8 +18,8 @@ pub fn compute_alive_vars(project: &Project) -> HashMap<Tid, HashSet<Variable>>
         .program
         .term
         .extern_symbols
-        .iter()
-        .map(|symbol| symbol.tid.clone())
+        .keys()
+        .cloned()
         .collect();
     let mut graph = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
     graph.reverse();

src/cwe_checker_lib/src/analysis/graph.rs

@@ -497,6 +497,7 @@ pub fn get_program_cfg(program: &Term<Program>, extern_subs: HashSet<Tid>) -> Gr
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::collections::BTreeMap;
 
     fn mock_program() -> Term<Program> {
         let call_term = Term {
@@ -577,7 +578,7 @@ mod tests {
             tid: Tid::new("program"),
             term: Program {
                 subs: vec![sub1, sub2],
-                extern_symbols: Vec::new(),
+                extern_symbols: BTreeMap::new(),
                 entry_points: Vec::new(),
                 address_base_offset: 0,
             },

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

@@ -26,7 +26,7 @@ pub struct Context<'a> {
     /// Note that values of writeable global memory segments are not tracked.
     pub runtime_memory_image: &'a RuntimeMemoryImage,
     /// Maps the TIDs of functions that shall be treated as extern symbols to the `ExternSymbol` object representing it.
-    pub extern_symbol_map: BTreeMap<Tid, &'a ExternSymbol>,
+    pub extern_symbol_map: &'a BTreeMap<Tid, ExternSymbol>,
     /// A channel where found CWE warnings and log messages should be sent to.
     /// The receiver may filter or modify the warnings before presenting them to the user.
     /// For example, the same CWE warning will be found several times
@@ -49,15 +49,11 @@ impl<'a> Context<'a> {
         config: Config,
         log_collector: crossbeam_channel::Sender<LogThreadMsg>,
     ) -> Context<'a> {
-        let mut extern_symbol_map = BTreeMap::new();
-        for symbol in project.program.term.extern_symbols.iter() {
-            extern_symbol_map.insert(symbol.tid.clone(), symbol);
-        }
         Context {
             graph: control_flow_graph,
             project,
             runtime_memory_image,
-            extern_symbol_map,
+            extern_symbol_map: &project.program.term.extern_symbols,
             log_collector,
             allocation_symbols: config.allocation_symbols,
             deallocation_symbols: config.deallocation_symbols,

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

@@ -14,21 +14,25 @@ fn new_id(time: &str, reg_name: &str) -> AbstractIdentifier {
     )
 }
 
-fn mock_extern_symbol(name: &str) -> ExternSymbol {
+fn mock_extern_symbol(name: &str) -> (Tid, ExternSymbol) {
     let arg = Arg::Register {
         var: register("RDX"),
         data_type: None,
     };
-    ExternSymbol {
-        tid: Tid::new("extern_".to_string() + name),
-        addresses: vec![],
-        name: name.into(),
-        calling_convention: None,
-        parameters: vec![arg.clone()],
-        return_values: vec![arg],
-        no_return: false,
-        has_var_args: false,
-    }
+    let tid = Tid::new("extern_".to_string() + name);
+    (
+        tid.clone(),
+        ExternSymbol {
+            tid,
+            addresses: vec![],
+            name: name.into(),
+            calling_convention: None,
+            parameters: vec![arg.clone()],
+            return_values: vec![arg],
+            no_return: false,
+            has_var_args: false,
+        },
+    )
 }
 
 fn register(name: &str) -> Variable {
@@ -77,7 +81,9 @@ fn mock_project() -> (Project, Config) {
             mock_extern_symbol("malloc"),
             mock_extern_symbol("free"),
             mock_extern_symbol("other"),
-        ],
+        ]
+        .into_iter()
+        .collect(),
         entry_points: Vec::new(),
         address_base_offset: 0,
     };

src/cwe_checker_lib/src/analysis/pointer_inference/mod.rs

@@ -267,13 +267,7 @@ impl<'a> PointerInference<'a> {
         // TODO: Refactor the fixpoint computation structs, so that the project reference can be extracted from them.
         let mut start_block_to_sub_map: HashMap<&Tid, &Term<Sub>> = HashMap::new();
         for sub in project.program.term.subs.iter() {
-            if project
-                .program
-                .term
-                .extern_symbols
-                .iter()
-                .any(|symbol| symbol.tid == sub.tid)
-            {
+            if project.program.term.extern_symbols.contains_key(&sub.tid) {
                 continue; // We ignore functions marked as extern symbols.
             }
             if let Some(start_block) = sub.term.blocks.first() {

src/cwe_checker_lib/src/checkers/cwe_367.rs

@@ -81,7 +81,7 @@ pub fn check_cwe(
         .term
         .extern_symbols
         .iter()
-        .map(|symbol| (symbol.name.as_str(), symbol.tid.clone()))
+        .map(|(tid, symbol)| (symbol.name.as_str(), tid.clone()))
         .collect();
 
     for (source, sink) in config.pairs {

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

@@ -87,8 +87,8 @@ impl<'a> Context<'a> {
             }
         }
         let mut extern_symbol_map = HashMap::new();
-        for symbol in project.program.term.extern_symbols.iter() {
-            extern_symbol_map.insert(symbol.tid.clone(), symbol);
+        for (tid, symbol) in project.program.term.extern_symbols.iter() {
+            extern_symbol_map.insert(tid.clone(), symbol);
         }
         Context {
             project,
@@ -112,14 +112,8 @@ impl<'a> Context<'a> {
                 .program
                 .term
                 .extern_symbols
-                .iter()
-                .find_map(|symb| {
-                    if symb.tid == *target {
-                        Some(symb.name.clone())
-                    } else {
-                        None
-                    }
-                })
+                .get(target)
+                .map(|symbol| symbol.name.clone())
                 .unwrap_or_else(|| "Unknown".to_string()),
             _ => "Unknown".to_string(),
         };

src/cwe_checker_lib/src/checkers/cwe_676.rs

@@ -19,7 +19,7 @@ False Negatives
 * None known
 */
 use crate::prelude::*;
-use std::collections::HashMap;
+use std::collections::{BTreeMap, HashMap, HashSet};
 
 use crate::{
     intermediate_representation::{ExternSymbol, Program, Sub, Term, Tid},
@@ -48,15 +48,11 @@ pub struct Config {
 /// For each subroutine and each found dangerous symbol, check for calls to the corresponding symbol
 pub fn get_calls<'a>(
     subfunctions: &'a [Term<Sub>],
-    dangerous_symbols: &'a [&'a ExternSymbol],
+    dangerous_symbols: &'a HashMap<&'a Tid, &'a str>,
 ) -> Vec<(&'a str, &'a Tid, &'a str)> {
     let mut calls: Vec<(&str, &Tid, &str)> = Vec::new();
-    let mut symbol_map: HashMap<&Tid, &str> = HashMap::with_capacity(dangerous_symbols.len());
-    for symbol in dangerous_symbols.iter() {
-        symbol_map.insert(&symbol.tid, symbol.name.as_str());
-    }
     for sub in subfunctions.iter() {
-        calls.append(&mut get_calls_to_symbols(sub, &symbol_map));
+        calls.append(&mut get_calls_to_symbols(sub, dangerous_symbols));
     }
 
     calls
@@ -95,15 +91,16 @@ pub fn generate_cwe_warnings<'a>(
 
 /// Filter external symbols by dangerous symbols
 pub fn resolve_symbols<'a>(
-    external_symbols: &'a [ExternSymbol],
+    external_symbols: &'a BTreeMap<Tid, ExternSymbol>,
     symbols: &'a [String],
-) -> Vec<&'a ExternSymbol> {
+) -> HashMap<&'a Tid, &'a str> {
+    let dangerous_symbols: HashSet<&'a String> = symbols.iter().collect();
     external_symbols
         .iter()
-        .filter(|symbol| {
-            symbols
-                .iter()
-                .any(|dangerous_function| *symbol.name == *dangerous_function)
+        .filter_map(|(tid, symbol)| {
+            dangerous_symbols
+                .get(&symbol.name)
+                .map(|name| (tid, name.as_str()))
         })
         .collect()
 }
@@ -118,7 +115,7 @@ pub fn check_cwe(
     let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
     let prog: &Term<Program> = &project.program;
     let subfunctions: &Vec<Term<Sub>> = &prog.term.subs;
-    let external_symbols: &Vec<ExternSymbol> = &prog.term.extern_symbols;
+    let external_symbols: &BTreeMap<Tid, ExternSymbol> = &prog.term.extern_symbols;
     let dangerous_symbols = resolve_symbols(external_symbols, &config.symbols);
     let dangerous_calls = get_calls(subfunctions, &dangerous_symbols);
 

src/cwe_checker_lib/src/checkers/cwe_78.rs

@@ -232,8 +232,8 @@ impl<'a> SymbolMaps<'a> {
     /// Creates a new instance of the symbol maps struct.
     pub fn new(project: &'a Project, config: &Config) -> Self {
         let mut extern_symbol_map = HashMap::new();
-        for symbol in project.program.term.extern_symbols.iter() {
-            extern_symbol_map.insert(symbol.tid.clone(), symbol);
+        for (tid, symbol) in project.program.term.extern_symbols.iter() {
+            extern_symbol_map.insert(tid.clone(), symbol);
         }
         SymbolMaps {
             string_symbol_map: crate::utils::symbol_utils::get_symbol_map(

src/cwe_checker_lib/src/checkers/cwe_78/context/tests.rs

@@ -103,12 +103,12 @@ impl Setup {
             .program
             .term
             .extern_symbols
-            .push(ExternSymbol::mock_string());
+            .insert(ExternSymbol::mock_string().tid, ExternSymbol::mock_string());
         project
             .program
             .term
             .extern_symbols
-            .push(ExternSymbol::mock());
+            .insert(ExternSymbol::mock().tid, ExternSymbol::mock());
         project.program.term.subs.push(sub);
         project.program.term.entry_points.push(Tid::new("func"));
         project.calling_conventions.push(CallingConvention::mock());
@@ -138,8 +138,8 @@ impl<'a> Context<'a> {
         graph.reverse();
 
         let mut extern_symbol_map = HashMap::new();
-        for symbol in project.program.term.extern_symbols.iter() {
-            extern_symbol_map.insert(symbol.tid.clone(), symbol);
+        for (tid, symbol) in project.program.term.extern_symbols.iter() {
+            extern_symbol_map.insert(tid.clone(), symbol);
         }
 
         let mut block_first_def_set: HashSet<(Tid, Tid)> = HashSet::new();

src/cwe_checker_lib/src/intermediate_representation/program.rs

 use super::{Blk, ExternSymbol, Sub};
 use crate::prelude::*;
+use std::collections::BTreeMap;
 
 /// The `Program` structure represents a disassembled binary.
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct Program {
     /// The known functions contained in the binary
     pub subs: Vec<Term<Sub>>,
     /// Extern symbols linked to the binary by the linker.
-    pub extern_symbols: Vec<ExternSymbol>,
+    pub extern_symbols: BTreeMap<Tid, ExternSymbol>,
     /// Entry points into to binary,
     /// i.e. the term identifiers of functions that may be called from outside of the binary.
     pub entry_points: Vec<Tid>,
@@ -42,7 +43,7 @@ mod tests {
         pub fn mock_empty() -> Program {
             Program {
                 subs: Vec::new(),
-                extern_symbols: Vec::new(),
+                extern_symbols: BTreeMap::new(),
                 entry_points: Vec::new(),
                 address_base_offset: 0,
             }

src/cwe_checker_lib/src/intermediate_representation/project.rs

@@ -10,7 +10,7 @@ use block_duplication_normalization::*;
 ///
 /// It contains information about the disassembled binary
 /// and about the execution environment of the binary.
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct Project {
     /// All (known) executable code of the binary is contained in the `program` term.
     pub program: Term<Program>,
@@ -89,8 +89,8 @@ impl Project {
                 jump_target_tids.insert(block.tid.clone());
             }
         }
-        for symbol in self.program.term.extern_symbols.iter() {
-            jump_target_tids.insert(symbol.tid.clone());
+        for symbol_tid in self.program.term.extern_symbols.keys() {
+            jump_target_tids.insert(symbol_tid.clone());
         }
         // Replace all jumps to non-existing jump targets with jumps to dummy targets
         let dummy_sub_tid = Tid::new("Artificial Sink Sub");

src/cwe_checker_lib/src/pcode/term.rs

@@ -610,7 +610,12 @@ impl Program {
         let extern_symbols = self
             .extern_symbols
             .into_iter()
-            .map(|symbol| symbol.into_ir_symbol(conventions, stack_pointer, cpu_arch))
+            .map(|symbol| {
+                (
+                    symbol.tid.clone(),
+                    symbol.into_ir_symbol(conventions, stack_pointer, cpu_arch),
+                )
+            })
             .collect();
         let address_base_offset =
             u64::from_str_radix(&self.image_base, 16).unwrap() - binary_base_address;

src/cwe_checker_lib/src/utils/symbol_utils.rs

@@ -8,9 +8,12 @@ use crate::intermediate_representation::*;
 /// Find the extern symbol object for a symbol name and return the symbol tid and name.
 pub fn find_symbol<'a>(prog: &'a Term<Program>, name: &str) -> Option<(&'a Tid, &'a str)> {
     let mut symbol: Option<(&'a Tid, &'a str)> = None;
-    prog.term.extern_symbols.iter().for_each(|sym| {
+    prog.term.extern_symbols.iter().find(|(_tid, sym)| {
         if name == sym.name {
             symbol = Some((&sym.tid, &sym.name));
+            true
+        } else {
+            false
         }
     });
 
@@ -46,18 +49,19 @@ pub fn get_symbol_map<'a>(
 ) -> HashMap<Tid, &'a ExternSymbol> {
     let mut tid_map = HashMap::new();
     for symbol_name in symbols_to_find {
-        if let Some((tid, symbol)) = project
-            .program
-            .term
-            .extern_symbols
-            .iter()
-            .find_map(|symbol| {
-                if symbol.name == *symbol_name {
-                    Some((symbol.tid.clone(), symbol))
-                } else {
-                    None
-                }
-            })
+        if let Some((tid, symbol)) =
+            project
+                .program
+                .term
+                .extern_symbols
+                .iter()
+                .find_map(|(_tid, symbol)| {
+                    if symbol.name == *symbol_name {
+                        Some((symbol.tid.clone(), symbol))
+                    } else {
+                        None
+                    }
+                })
         {
             tid_map.insert(tid, symbol);
         }