Generate context information for CWE-416 warnings (#408)

Dockerfile

-FROM rust:1.63 AS builder
+FROM rust:1.69 AS builder
 
 WORKDIR /cwe_checker
 

README.md

@@ -55,7 +55,7 @@ The prebuilt Docker images are currently only x86-based.
 ### Local installation ###
 
 The following dependencies must be installed in order to build and install the *cwe_checker* locally:
--   [Rust](https://www.rust-lang.org) >= 1.63
+-   [Rust](https://www.rust-lang.org) >= 1.69
 -   [Ghidra](https://ghidra-sre.org/) >= 10.2 (**Warning:** This applies to the master branch, the v0.6 stable release needs Ghidra 10.1.5)
 
 Run `make all GHIDRA_PATH=/path/to/ghidra_folder` (with the correct path to the local Ghidra installation inserted) to compile and install the cwe_checker.

src/cwe_checker_lib/src/abstract_domain/identifier.rs

@@ -105,6 +105,16 @@ impl AbstractIdentifier {
         }
     }
 
+    /// Create a new abstract identifier by removing the last path hint from the path hint array of `self`.
+    /// Return the new identifier together with the removed path hint (or none if `self` has no path hints).
+    pub fn without_last_path_hint(&self) -> (Self, Option<Tid>) {
+        let mut new_id = self.clone();
+        let inner = Arc::make_mut(&mut new_id.0);
+        let last_path_hint = inner.path_hints.pop();
+
+        (new_id, last_path_hint)
+    }
+
     /// Get the path hints array of `self`.
     pub fn get_path_hints(&self) -> &[Tid] {
         &self.path_hints

src/cwe_checker_lib/src/analysis/string_abstraction/context/symbol_calls/sprintf/tests.rs

@@ -261,7 +261,7 @@ fn test_push_constant_subsequences_before_and_between_specifiers() {
     let mut specifier_ends: Vec<usize> = vec![0];
     specifier_ends.push(matches.get(0).unwrap().end());
 
-    for (index, (mat, spec_end)) in itertools::zip(matches, specifier_ends)
+    for (index, (mat, spec_end)) in std::iter::zip(matches, specifier_ends)
         .into_iter()
         .enumerate()
     {

src/cwe_checker_lib/src/checkers/cwe_416/state.rs

@@ -25,8 +25,8 @@ impl AbstractDomain for ObjectState {
             (ObjectState::AlreadyFlagged, _) | (_, ObjectState::AlreadyFlagged) => {
                 ObjectState::AlreadyFlagged
             }
-            (ObjectState::Dangling(tid), ObjectState::Dangling(_)) => {
-                ObjectState::Dangling(tid.clone())
+            (ObjectState::Dangling(tid), ObjectState::Dangling(other_tid)) => {
+                ObjectState::Dangling(std::cmp::min(tid, other_tid).clone())
             }
         }
     }
@@ -54,17 +54,34 @@ impl State {
         }
     }
 
+    /// Return whether the given object ID is already flagged in this state,
+    /// i.e. whether a CWE warning was already generated for this object.
+    pub fn is_id_already_flagged(&self, object_id: &AbstractIdentifier) -> bool {
+        self.dangling_objects.get(object_id) == Some(&ObjectState::AlreadyFlagged)
+    }
+
+    /// If the given `object_id` represents a dangling object, return the TID of the site where it was freed.
+    pub fn get_free_tid_if_dangling(&self, object_id: &AbstractIdentifier) -> Option<&Tid> {
+        if let Some(ObjectState::Dangling(free_tid)) = self.dangling_objects.get(object_id) {
+            Some(free_tid)
+        } else {
+            None
+        }
+    }
+
     /// Check the given address on whether it may point to already freed memory.
-    /// For each possible dangling pointer target a string describing the root cause is returnen.
+    /// For each possible dangling pointer target the abstract ID of the object
+    /// and the TID of the corresponding site where the object was freed is returned.
     /// The object states of corresponding memory objects are set to [`ObjectState::AlreadyFlagged`]
     /// to prevent reporting duplicate CWE messages with the same root cause.
-    pub fn check_address_for_use_after_free(&mut self, address: &Data) -> Option<Vec<String>> {
+    pub fn check_address_for_use_after_free(
+        &mut self,
+        address: &Data,
+    ) -> Option<Vec<(AbstractIdentifier, Tid)>> {
         let mut free_ids_of_dangling_pointers = Vec::new();
         for id in address.get_relative_values().keys() {
             if let Some(ObjectState::Dangling(free_id)) = self.dangling_objects.get(id) {
-                free_ids_of_dangling_pointers.push(format!(
-                    "Accessed ID {id} may have been already freed at {free_id}"
-                ));
+                free_ids_of_dangling_pointers.push((id.clone(), free_id.clone()));
 
                 self.dangling_objects
                     .insert(id.clone(), ObjectState::AlreadyFlagged);
@@ -84,7 +101,7 @@ impl State {
         call_tid: &Tid,
         param: &Data,
         pi_state: &PiState,
-    ) -> Option<Vec<String>> {
+    ) -> Option<Vec<(AbstractIdentifier, Tid)>> {
         // FIXME: This function could also generate debug log messages whenever nonsensical information is detected.
         // E.g. stack frame IDs or non-zero ID offsets can be indicators of other bugs.
         let mut warnings = Vec::new();
@@ -99,9 +116,7 @@ impl State {
                 .dangling_objects
                 .insert(id.clone(), ObjectState::Dangling(call_tid.clone()))
             {
-                warnings.push(format!(
-                    "Object {id} may have been freed before at {old_free_id}."
-                ));
+                warnings.push((id.clone(), old_free_id.clone()));
             }
         }
         if !warnings.is_empty() {
@@ -164,12 +179,61 @@ impl AbstractDomain for State {
     }
 }
 
+impl State {
+    /// Get a more compact json-representation of the state.
+    /// Intended for pretty printing, not useable for serialization/deserialization.
+    #[allow(dead_code)]
+    pub fn to_json_compact(&self) -> serde_json::Value {
+        use serde_json::*;
+        let mut state_map = Map::new();
+        state_map.insert(
+            "current_function".to_string(),
+            Value::String(format!("{}", self.current_fn_tid)),
+        );
+        for (id, object_state) in self.dangling_objects.iter() {
+            if let ObjectState::Dangling(free_tid) = object_state {
+                state_map.insert(
+                    format!("{id}"),
+                    Value::String(format!("Dangling({free_tid})")),
+                );
+            } else {
+                state_map.insert(
+                    format!("{id}"),
+                    Value::String("Already flagged".to_string()),
+                );
+            }
+        }
+        Value::Object(state_map)
+    }
+}
+
 #[cfg(test)]
 pub mod tests {
     use super::*;
     use crate::{bitvec, intermediate_representation::parsing, variable};
     use std::collections::BTreeSet;
 
+    impl State {
+        pub fn mock(
+            current_fn_tid: Tid,
+            dangling_ids: &[(AbstractIdentifier, Tid)],
+            already_flagged_ids: &[AbstractIdentifier],
+        ) -> Self {
+            let mut state = State::new(current_fn_tid);
+            for (id, free_id) in dangling_ids.iter() {
+                state
+                    .dangling_objects
+                    .insert(id.clone(), ObjectState::Dangling(free_id.clone()));
+            }
+            for id in already_flagged_ids.iter() {
+                state
+                    .dangling_objects
+                    .insert(id.clone(), ObjectState::AlreadyFlagged);
+            }
+            state
+        }
+    }
+
     #[test]
     fn test_check_address_for_use_after_free() {
         let mut state = State::new(Tid::new("current_fn"));

src/cwe_checker_lib/src/intermediate_representation/macros/mod.rs

@@ -193,7 +193,7 @@ pub mod parsing {
             r"^((0x(-)?[[:alnum:]]+)|^(-)?([0-9])+)+:[0-9]+$",                     // Constant
             r"^[^\+]*\+{1}[^\+]*$",                                                // BinOp (IntAdd)
             r"^[[:ascii:]]+ \-{1} [[:ascii:]]+$",                                  // BinOp (IntSub)
-            r"^-\([[:ascii:]]*\)$",  // UnOp (IntNegate)
+            r"^-\([[:ascii:]]*\)$", // UnOp (IntNegate)
             r"^¬\([[:ascii:]]*\)$", // UnOp (BoolNegate)
         ])
         .unwrap();