Handle pointer comparisons in specialize_conditional (#180)

src/cwe_checker_lib/src/abstract_domain/pointer.rs

@@ -122,6 +122,15 @@ impl<T: RegisterDomain> PointerDomain<T> {
     pub fn ids(&self) -> std::collections::btree_map::Keys<AbstractIdentifier, T> {
         self.0.keys()
     }
+
+    /// Return the target and offset of the pointer if it points to an unique ID.
+    pub fn unwrap_if_unique_target(&self) -> Option<(&AbstractIdentifier, &T)> {
+        if self.0.len() == 1 {
+            return self.0.iter().next();
+        } else {
+            None
+        }
+    }
 }
 
 impl<T: RegisterDomain + Display> PointerDomain<T> {

src/cwe_checker_lib/src/analysis/pointer_inference/object_list.rs

@@ -363,8 +363,8 @@ impl AbstractObjectList {
         }
     }
 
-    // Return the object type of a memory object.
-    // Returns an error if no object with the given ID is contained in the object list.
+    /// Return the object type of a memory object.
+    /// Returns an error if no object with the given ID is contained in the object list.
     pub fn get_object_type(
         &self,
         object_id: &AbstractIdentifier,
@@ -374,6 +374,16 @@ impl AbstractObjectList {
             None => Err(()),
         }
     }
+
+    /// Returns `true` if the object corresponding to the given ID represents an unique object
+    /// and `false` if it may represent more than one object (e.g. several array elements).
+    /// Returns an error if the ID is not contained in the object list.
+    pub fn is_unique_object(&self, object_id: &AbstractIdentifier) -> Result<bool, Error> {
+        match self.objects.get(object_id) {
+            Some((object, _)) => Ok(object.is_unique),
+            None => Err(anyhow!("Object ID not contained in object list.")),
+        }
+    }
 }
 
 impl AbstractDomain for AbstractObjectList {

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

@@ -525,6 +525,8 @@ impl State {
                             if let Ok(bitvec) = self.eval(rhs).try_to_bitvec() {
                                 self.specialize_by_expression_result(lhs, bitvec.into())?;
                             }
+                            // Also specialize cases of pointer comparisons
+                            self.specialize_pointer_comparison(&BinOpType::IntEqual, lhs, rhs)?;
                             Ok(())
                         }
                         (BinOpType::IntEqual, false) | (BinOpType::IntNotEqual, true) => {
@@ -537,6 +539,8 @@ impl State {
                                 let new_result = self.eval(lhs).add_not_equal_bound(&bitvec)?;
                                 self.specialize_by_expression_result(lhs, new_result)?;
                             }
+                            // Also specialize cases of pointer comparisons
+                            self.specialize_pointer_comparison(&BinOpType::IntNotEqual, lhs, rhs)?;
                             Ok(())
                         }
                         _ => panic!(),
@@ -583,6 +587,60 @@ impl State {
         }
     }
 
+    /// If both `lhs` and `rhs` evaluate to pointers and `op` is a comparison operator that evaluates to `true`,
+    /// specialize the input pointers accordingly.
+    ///
+    /// Note that the current implementation only specializes for `==` and `!=` operators
+    /// and only if the pointers point to the same unique memory object.
+    fn specialize_pointer_comparison(
+        &mut self,
+        op: &BinOpType,
+        lhs: &Expression,
+        rhs: &Expression,
+    ) -> Result<(), Error> {
+        if let (Data::Pointer(lhs_pointer), Data::Pointer(rhs_pointer)) =
+            (self.eval(lhs), self.eval(rhs))
+        {
+            match (
+                lhs_pointer.unwrap_if_unique_target(),
+                rhs_pointer.unwrap_if_unique_target(),
+            ) {
+                (Some((lhs_id, lhs_offset)), Some((rhs_id, rhs_offset))) if lhs_id == rhs_id => {
+                    if !(self.memory.is_unique_object(lhs_id)?) {
+                        // Since the pointers may or may not point to different instances referenced by the same ID we cannot compare them.
+                        return Ok(());
+                    }
+                    if *op == BinOpType::IntEqual {
+                        let specialized_offset = lhs_offset.intersect(rhs_offset)?;
+                        let specialized_domain: Data =
+                            PointerDomain::new(lhs_id.clone(), specialized_offset).into();
+                        self.specialize_by_expression_result(lhs, specialized_domain.clone())?;
+                        self.specialize_by_expression_result(rhs, specialized_domain)?;
+                    } else if *op == BinOpType::IntNotEqual {
+                        if let Ok(rhs_offset_bitvec) = rhs_offset.try_to_bitvec() {
+                            let new_lhs_offset =
+                                lhs_offset.clone().add_not_equal_bound(&rhs_offset_bitvec)?;
+                            self.specialize_by_expression_result(
+                                lhs,
+                                PointerDomain::new(lhs_id.clone(), new_lhs_offset).into(),
+                            )?;
+                        }
+                        if let Ok(lhs_offset_bitvec) = lhs_offset.try_to_bitvec() {
+                            let new_rhs_offset =
+                                rhs_offset.clone().add_not_equal_bound(&lhs_offset_bitvec)?;
+                            self.specialize_by_expression_result(
+                                rhs,
+                                PointerDomain::new(rhs_id.clone(), new_rhs_offset).into(),
+                            )?;
+                        }
+                    }
+                }
+                _ => (), // Other cases not handled, since it depends on the meaning of pointer IDs, which may change in the future.
+            }
+        }
+        Ok(())
+    }
+
     /// Try to restrict the input variables of the given comparison operation
     /// (signed and unsigned versions of `<` and `<=`)
     /// so that the comparison evaluates to `true`.

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

@@ -1066,3 +1066,34 @@ fn out_of_bounds_access_recognition() {
     state.set_register(&Variable::mock("RAX", 8), address);
     assert!(!state.contains_out_of_bounds_mem_access(&load_def.term, &global_data));
 }
+
+#[test]
+fn specialize_pointer_comparison() {
+    let mut state = State::new(&register("RSP"), Tid::new("func_tid"));
+    let interval = IntervalDomain::mock(-5, 10);
+    state.set_register(
+        &register("RAX"),
+        PointerDomain::new(new_id("func_tid", "RSP"), interval.into()).into(),
+    );
+    let interval = IntervalDomain::mock(20, 20);
+    state.set_register(
+        &register("RBX"),
+        PointerDomain::new(new_id("func_tid", "RSP"), interval.into()).into(),
+    );
+    let expression = Expression::BinOp {
+        op: BinOpType::IntEqual,
+        lhs: Box::new(Expression::Var(register("RAX"))),
+        rhs: Box::new(Expression::Var(register("RBX"))),
+    };
+    assert!(state
+        .clone()
+        .specialize_by_expression_result(&expression, Bitvector::from_i8(1).into())
+        .is_err());
+    let specialized_interval = IntervalDomain::mock_with_bounds(None, -5, 10, Some(19));
+    let specialized_pointer =
+        PointerDomain::new(new_id("func_tid", "RSP"), specialized_interval.into()).into();
+    assert!(state
+        .specialize_by_expression_result(&expression, Bitvector::from_i8(0).into())
+        .is_ok());
+    assert_eq!(state.get_register(&register("RAX")), specialized_pointer);
+}

test/src/lib.rs

@@ -229,17 +229,15 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_119", "Memory");
 
-        mark_skipped(&mut tests, "mips64", "gcc"); // TODO: Check reason for failure!
-        mark_skipped(&mut tests, "mips64el", "gcc"); // TODO: Check reason for failure!
-        mark_skipped(&mut tests, "mips", "clang"); // TODO: Check reason for failure!
-        mark_skipped(&mut tests, "mipsel", "clang"); // TODO: Check reason for failure!
+        mark_architecture_skipped(&mut tests, "mips"); // A second unrelated instance is found in "__do_global_ctors_aux".
+        mark_architecture_skipped(&mut tests, "mipsel"); // A second unrelated instance is found in "__do_global_ctors_aux".
 
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
         mark_skipped(&mut tests, "x86", "gcc"); // Loss of stack register value since we do not track pointer alignment yet.
+        mark_skipped(&mut tests, "x86", "clang"); // A second unrelated instance is found in "__do_global_ctors_aux".
 
-        mark_skipped(&mut tests, "x86", "clang"); // TODO: Check reason for failure!
         mark_compiler_skipped(&mut tests, "mingw32-gcc"); // TODO: Check reason for failure!
 
         for test_case in tests {
@@ -570,17 +568,17 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_119", "Memory");
 
-        mark_skipped(&mut tests, "arm", "gcc"); // TODO: Check reason for failure!
-        mark_skipped(&mut tests, "mips64", "gcc"); // TODO: Check reason for failure!
-        mark_skipped(&mut tests, "mips64el", "gcc"); // TODO: Check reason for failure!
+        mark_skipped(&mut tests, "arm", "gcc"); // Needs tracking of linear dependencies between register values.
+        mark_skipped(&mut tests, "mips64", "gcc"); // Needs tracking of linear dependencies between register values.
+        mark_skipped(&mut tests, "mips64el", "gcc"); // Needs tracking of linear dependencies between register values.
 
-        mark_architecture_skipped(&mut tests, "mips"); // TODO: Check reason for failure!
-        mark_architecture_skipped(&mut tests, "mipsel"); // TODO: Check reason for failure!
+        mark_architecture_skipped(&mut tests, "mips"); // Needs tracking of linear dependencies between register values.
+        mark_architecture_skipped(&mut tests, "mipsel"); // Needs tracking of linear dependencies between register values.
 
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
-        mark_skipped(&mut tests, "ppc", "gcc"); // TODO: Check reason for failure!
+        mark_skipped(&mut tests, "ppc", "gcc"); // Needs tracking of linear dependencies between register values.
 
         mark_skipped(&mut tests, "x86", "gcc"); // Loss of stack register value since we do not track pointer alignment yet.