test: fix acceptance tests

.github/workflows/acceptance-tests.yml

@@ -1,6 +1,6 @@
 name: Acceptance tests
 
-on: 
+on:
   push:
     branches:
       - master
@@ -10,16 +10,16 @@ on:
 
 env:
   CARGO_TERM_COLOR: always
-  GHIDRA_RELEASE_TAG: Ghidra_10.2.3_build
-  GHIDRA_VERSION: ghidra_10.2.3_PUBLIC_20230208
 
-jobs: 
- 
+jobs:
+
   acceptance-tests:
     runs-on: ubuntu-latest
 
     steps:
       - uses: actions/checkout@v3
+      - name: Build the cwe_checker docker image
+        run: docker build -t cwe_checker .
       - name: Build and run docker image for cross compiling
         run: |
           pushd test/artificial_samples
@@ -28,28 +28,27 @@ jobs:
           popd
           pushd test/lkm_samples
           ./build.sh
-      - uses: actions/setup-java@v1
-        with:
-          java-version: "17.0.x"
-          java-package: jdk
-          architecture: x64
-      - name: Install Ghidra
-        run: |
-          wget https://github.com/NationalSecurityAgency/ghidra/releases/download/${GHIDRA_RELEASE_TAG}/${GHIDRA_VERSION}.zip
-          unzip -d ghidra ${GHIDRA_VERSION}.zip
-          mv ghidra/ghidra_* /opt/ghidra
-          rm ${GHIDRA_VERSION}.zip
       - uses: actions-rs/toolchain@v1
         with:
           profile: minimal
           toolchain: 1.82.0
           override: true
-      - name: Install cwe_checker
-        run: make all GHIDRA_PATH=/opt/ghidra
       - uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --locked --no-fail-fast -p acceptance_tests_ghidra -- --show-output --ignored --test-threads 1
+          args: --locked --no-fail-fast -p acceptance_tests_ghidra -F docker -- --show-output --ignored
+      - name: Generate zip with test binaries
+        run: |
+          zip artificial_samples.zip test/artificial_samples/build/*.out
+          zip lkm_samples.zip test/lkm_samples/build/*.ko
+      - name: Archive test binaries
+        uses: actions/upload-artifact@v4
+        with:
+          name: acceptance-test-binaries
+          path: |
+            artificial_samples.zip
+            lkm_samples.zip
+
 
   docker-build:
     runs-on: ubuntu-latest

src/cwe_checker_lib/src/checkers/cwe_134.rs

@@ -78,6 +78,8 @@ pub fn check_cwe(
     analysis_results: &AnalysisResults,
     cwe_params: &serde_json::Value,
 ) -> WithLogs<Vec<CweWarning>> {
+    let mut logs = Vec::new();
+
     let project = analysis_results.project;
     let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
     let format_string_symbols =
@@ -104,13 +106,24 @@ pub fn check_cwe(
                         StringLocation::GlobalWriteable | StringLocation::NonGlobal
                     ) {
                         cwe_warnings.push(generate_cwe_warning(&jmp.tid, symbol, &location));
+                    } else if matches!(location, StringLocation::Unknown) {
+                        logs.push(LogMessage::new_debug(format!(
+                            "{}: No PI result for call at {}.",
+                            CWE_MODULE.name, jmp.tid
+                        )));
                     }
                 }
             }
         }
     }
 
-    WithLogs::wrap(cwe_warnings)
+    WithLogs::new(
+        cwe_warnings
+            .deduplicate_first_address()
+            .move_logs_to(&mut logs)
+            .into_object(),
+        logs,
+    )
 }
 
 /// Returns a StringLocation based on the kind of memory
@@ -138,17 +151,22 @@ fn locate_format_string(
                         .is_address_writeable(&address_vector)
                         .unwrap()
                     {
-                        return StringLocation::GlobalWriteable;
+                        StringLocation::GlobalWriteable
+                    } else {
+                        StringLocation::GlobalReadable
                     }
-
-                    return StringLocation::GlobalReadable;
+                } else {
+                    StringLocation::NonGlobal
                 }
+            } else {
+                StringLocation::NonGlobal
             }
+        } else {
+            StringLocation::NonGlobal
         }
-        return StringLocation::NonGlobal;
+    } else {
+        StringLocation::Unknown
     }
-
-    StringLocation::Unknown
 }
 
 /// Generate the CWE warning for a detected instance of the CWE.

src/cwe_checker_lib/src/checkers/cwe_190.rs

@@ -140,12 +140,16 @@ fn contains_only_non_top_absolute_value(data_domain: &DataDomain<IntervalDomain>
 }
 
 /// Run the CWE check.
+///
 /// For each call to one of the symbols configured in config.json
-/// we check whether the block containing the call also contains a multiplication instruction.
+/// we check whether the block containing the call also contains a
+/// multiplication instruction.
 pub fn check_cwe(
     analysis_results: &AnalysisResults,
     cwe_params: &serde_json::Value,
 ) -> WithLogs<Vec<CweWarning>> {
+    let mut logs = Vec::new();
+
     let project = analysis_results.project;
     let pointer_inference_results = analysis_results.pointer_inference.unwrap();
 
@@ -177,5 +181,11 @@ pub fn check_cwe(
         }
     }
 
-    WithLogs::wrap(cwe_warnings)
+    WithLogs::new(
+        cwe_warnings
+            .deduplicate_first_address()
+            .move_logs_to(&mut logs)
+            .into_object(),
+        logs,
+    )
 }

src/cwe_checker_lib/src/checkers/cwe_243.rs

@@ -177,5 +177,5 @@ pub fn check_cwe(
         }
     }
 
-    WithLogs::wrap(cwe_warnings)
+    cwe_warnings.deduplicate_first_address()
 }

src/cwe_checker_lib/src/checkers/cwe_252.rs

@@ -72,7 +72,7 @@ use crate::CweModule;
 
 use petgraph::visit::EdgeRef;
 
-use std::collections::{BTreeMap, HashSet, VecDeque};
+use std::collections::{HashSet, VecDeque};
 use std::sync::Arc;
 
 mod context;
@@ -228,16 +228,12 @@ impl<'a, 'b: 'a> CweAnalysis<'a, 'b> {
             isolated_returns.analyze(&ta_comp);
         }
 
-        WithLogs::wrap(
-            self.cwe_collector
-                .try_iter()
-                // FIXME: It would be nice to preerve all reasons during
-                // deduplication.
-                .map(|msg| (msg.tids.clone(), msg))
-                .collect::<BTreeMap<_, _>>()
-                .into_values()
-                .collect(),
-        )
+        self.cwe_collector
+            .try_iter()
+            .collect::<Vec<_>>()
+            // FIXME: It would be nice to preserve all reasons during
+            // deduplication.
+            .deduplicate_addresses()
     }
 }
 

src/cwe_checker_lib/src/checkers/cwe_367.rs

@@ -120,5 +120,5 @@ pub fn check_cwe(
         }
     }
 
-    WithLogs::wrap(cwe_warnings)
+    cwe_warnings.deduplicate_addresses()
 }

src/cwe_checker_lib/src/checkers/cwe_416/mod.rs

@@ -150,7 +150,16 @@ pub fn check_cwe(
         logs.insert(log_msg);
     }
 
-    WithLogs::new(cwes.into_iter().collect(), logs.into_iter().collect())
+    let cwe_warnings: Vec<CweWarning> = cwes.into_iter().collect();
+    let mut logs: Vec<LogMessage> = logs.into_iter().collect();
+
+    WithLogs::new(
+        cwe_warnings
+            .deduplicate_first_address()
+            .move_logs_to(&mut logs)
+            .into_object(),
+        logs,
+    )
 }
 
 /// A struct for collecting CWE warnings together with context information

src/cwe_checker_lib/src/checkers/cwe_426.rs

@@ -100,5 +100,5 @@ pub fn check_cwe(
         }
     }
 
-    WithLogs::wrap(cwe_warnings)
+    cwe_warnings.deduplicate_first_address()
 }

src/cwe_checker_lib/src/checkers/cwe_467.rs

@@ -116,13 +116,13 @@ pub fn check_cwe(
     let mut cwe_warnings = Vec::new();
 
     let symbol_map = get_symbol_map(project, &config.symbols);
-    for sub in project.program.term.subs.values() {
-        for (block, jmp, symbol) in get_callsites(sub, &symbol_map) {
+    for f in project.program.functions() {
+        for (block, jmp, symbol) in get_callsites(f, &symbol_map) {
             if check_for_pointer_sized_arg(project, block, symbol) {
                 cwe_warnings.push(generate_cwe_warning(jmp, symbol))
             }
         }
     }
 
-    WithLogs::wrap(cwe_warnings)
+    cwe_warnings.deduplicate_first_address()
 }

src/cwe_checker_lib/src/checkers/cwe_560.rs

@@ -137,5 +137,10 @@ pub fn check_cwe(
         }
     }
 
-    WithLogs::new(cwes, log_messages)
+    WithLogs::new(
+        cwes.deduplicate_first_address()
+            .move_logs_to(&mut log_messages)
+            .into_object(),
+        log_messages,
+    )
 }

src/cwe_checker_lib/src/checkers/cwe_676.rs

@@ -103,19 +103,37 @@ pub fn resolve_symbols<'a>(
         .collect()
 }
 
-/// Iterate through all function calls inside the program and flag calls to those functions
-/// that are marked as unsafe via the configuration file.
+/// Iterate through all function calls inside the program and flag calls to
+/// those functions that are marked as unsafe via the configuration file.
 pub fn check_cwe(
     analysis_results: &AnalysisResults,
     cwe_params: &serde_json::Value,
 ) -> WithLogs<Vec<CweWarning>> {
+    let mut logs = Vec::new();
+
     let project = analysis_results.project;
     let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
-    let prog: &Term<Program> = &project.program;
-    let subfunctions = &prog.term.subs;
-    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);
+    let prog = &project.program;
+    let functions = &prog.term.subs;
+    let external_symbols = &prog.term.extern_symbols;
+
+    let dangerous_ext_symbols = filter_dangerous_ext_symbols(external_symbols, &config.symbols);
+    let mut msg = format!(
+        "{}: Program imports the following dangerous symbols: ",
+        CWE_MODULE.name
+    );
+    for (fn_tid, fn_name) in dangerous_ext_symbols.iter() {
+        msg.push_str(&format!("{}({}),", fn_tid, fn_name));
+    }
+    logs.push(LogMessage::new_info(msg));
+
+    let dangerous_ext_calls = get_calls(functions, &dangerous_ext_symbols);
 
-    WithLogs::wrap(generate_cwe_warnings(dangerous_calls))
+    WithLogs::new(
+        generate_cwe_warnings(dangerous_ext_calls)
+            .deduplicate_first_address()
+            .move_logs_to(&mut logs)
+            .into_object(),
+        logs,
+    )
 }

src/cwe_checker_lib/src/checkers/cwe_789.rs

@@ -133,31 +133,49 @@ pub fn check_cwe(
     analysis_results: &AnalysisResults,
     cwe_params: &serde_json::Value,
 ) -> WithLogs<Vec<CweWarning>> {
+    let mut logs = Vec::new();
     let project = analysis_results.project;
     let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
     let mut cwe_warnings = Vec::new();
     let pir = analysis_results.pointer_inference.unwrap();
     let symbol_map = get_symbol_map(project, &config.symbols);
 
-    'functions: for sub in project.program.term.subs.values() {
+    'functions: for f in project.program.functions() {
         // Function call allocation case
-        for (_, jump, symbol) in get_callsites(sub, &symbol_map) {
-            if let Some(interval) = match symbol.name.as_str() {
+        for (_, jump, symbol) in get_callsites(f, &symbol_map) {
+            if let Some(alloc_size_interval) = match symbol.name.as_str() {
                 "calloc" => multiply_args_for_calloc(
                     pir,
                     &jump.tid,
                     vec![&symbol.parameters[0], &symbol.parameters[1]],
                 ),
                 "realloc" => pir.eval_parameter_arg_at_call(&jump.tid, &symbol.parameters[1]),
-                _ => pir.eval_parameter_arg_at_call(&jump.tid, &symbol.parameters[0]),
+                "malloc" => pir.eval_parameter_arg_at_call(&jump.tid, &symbol.parameters[0]),
+                name => {
+                    logs.push(LogMessage::new_info(format!(
+                        "{}: Allocation size computation for calls to {} is not supported (at {}: {}).",
+                        CWE_MODULE.name, name, jump.tid, jump.term
+                    )));
+                    None
+                }
             } {
-                if exceeds_threshold_on_call(interval, config.heap_threshold) {
+                logs.push(LogMessage::new_debug(format!(
+                    "{}: Allocation size interval is {:?} for call to {} at {}: {}.",
+                    CWE_MODULE.name, alloc_size_interval, &symbol.name, jump.tid, jump.term
+                )));
+
+                if exceeds_threshold_on_call(alloc_size_interval, config.heap_threshold) {
                     cwe_warnings.push(generate_cwe_warning(&jump.tid, false));
                 }
+            } else {
+                logs.push(LogMessage::new_debug(format!(
+                    "{}: Unable to compute allocation size of call to {} at {}: {}.",
+                    CWE_MODULE.name, &symbol.name, jump.tid, jump.term
+                )));
             }
         }
         // Stack allocation case
-        for blk in &sub.term.blocks {
+        for blk in &f.term.blocks {
             let assign_on_sp: Vec<&Term<Def>> = blk
                 .term
                 .defs
@@ -174,7 +192,12 @@ pub fn check_cwe(
             }
         }
     }
-    cwe_warnings.dedup();
 
-    WithLogs::wrap(cwe_warnings)
+    WithLogs::new(
+        cwe_warnings
+            .deduplicate_first_address()
+            .move_logs_to(&mut logs)
+            .into_object(),
+        logs,
+    )
 }