CWE-190: added Pointer Inference results as basis for overflow detection. (#336)

src/caller/src/main.rs

@@ -177,7 +177,7 @@ fn run_with_ghidra(args: &CmdlineArgs) {
 
     let modules_depending_on_string_abstraction = BTreeSet::from_iter(["CWE78"]);
     let modules_depending_on_pointer_inference =
-        BTreeSet::from_iter(["CWE119", "CWE134", "CWE416", "CWE476", "Memory"]);
+        BTreeSet::from_iter(["CWE119", "CWE134", "CWE416", "CWE476", "Memory", "CWE190"]);
 
     let string_abstraction_needed = modules
         .iter()

src/cwe_checker_lib/src/abstract_domain/data.rs

@@ -15,7 +15,7 @@ mod trait_impl;
 /// An abstract domain representing a set of base values plus offsets or an absolute value (or both).
 ///
 /// The base values are represented as abstract IDs,
-/// i.e. they are treated as variables with unknown absolute value.
+/// i.e. they are treated as variables with unknown absolute value, e.g. the returned pointer by malloc.
 /// For each base value the offset is given by an abstract domain `T`,
 /// which should specialize in representing absolute values (e.g. an interval domain).
 /// Note that the domain assumes pointer semantics for these values.

src/cwe_checker_lib/src/checkers/cwe_190.rs

 //! This module implements a check for CWE-190: Integer overflow or wraparound.
 //!
-//! An integer overflow can lead to undefined behaviour and is especially dangerous
+//! An integer overflow can lead to undefined behavior and is especially dangerous
 //! in conjunction with memory management functions.
 //!
 //! See <https://cwe.mitre.org/data/definitions/190.html> for a detailed description.
@@ -10,7 +10,8 @@
 //! For each call to a function from the CWE190 symbol list we check whether the
 //! basic block directly before the call contains a multiplication instruction.
 //! If one is found, the call gets flagged as a CWE hit, as there is no overflow
-//! check corresponding to the multiplication before the call. The default CWE190
+//! check corresponding to the multiplication before the call as well as
+//! the Pointer Inference can not exclude an overflow. The default CWE190
 //! symbol list contains the memory allocation functions *malloc*, *xmalloc*,
 //! *calloc* and *realloc*. The list is configurable in config.json.
 //!
@@ -19,8 +20,7 @@
 //! - There is no check whether the result of the multiplication is actually used
 //!   as input to the function call. However, this does not seem to generate a lot
 //!   of false positives in practice.
-//! - There is no value set analysis in place to determine whether an overflow is
-//!   possible or not at the specific instruction.
+//! - Values that are not absolute e.g. user controlled or depend on other values.
 //!
 //! ## False Negatives
 //!
@@ -28,6 +28,12 @@
 //! from the CWE190 symbol list.
 //! - All integer overflows caused by addition or subtraction.
 
+use crate::abstract_domain::AbstractDomain;
+use crate::abstract_domain::DataDomain;
+use crate::abstract_domain::IntervalDomain;
+use crate::abstract_domain::RegisterDomain;
+use crate::analysis::pointer_inference::*;
+use crate::analysis::vsa_results::*;
 use crate::intermediate_representation::*;
 use crate::prelude::*;
 use crate::utils::log::{CweWarning, LogMessage};
@@ -97,6 +103,41 @@ fn generate_cwe_warning(callsite: &Tid, called_symbol: &ExternSymbol) -> CweWarn
         .symbols(vec![called_symbol.name.clone()])
 }
 
+/// Determines if all parameters are only absolute values and their included intervals are not top valued.
+fn contains_top_value(pir: &PointerInference, jmp_tid: &Tid, parms: Vec<&Arg>) -> bool {
+    for arg in parms {
+        if let Some(value) = pir.eval_parameter_arg_at_call(jmp_tid, arg) {
+            if !contains_only_non_top_absolute_value(&value) {
+                return true;
+            }
+        }
+    }
+    false
+}
+
+/// Checks if the multiplication of element count and size parameters result in an overflow.
+fn calloc_parm_mul_is_top(pir: &PointerInference, jmp_tid: &Tid, parms: Vec<&Arg>) -> bool {
+    if let (Some(nmeb), Some(size)) = (
+        pir.eval_parameter_arg_at_call(jmp_tid, parms[0]),
+        pir.eval_parameter_arg_at_call(jmp_tid, parms[1]),
+    ) {
+        nmeb.bin_op(BinOpType::IntMult, &size);
+        return !contains_only_non_top_absolute_value(&nmeb);
+    }
+
+    false
+}
+
+/// Determines if the data domain only has absolute values and their included interval is not top valued.
+fn contains_only_non_top_absolute_value(data_domain: &DataDomain<IntervalDomain>) -> bool {
+    if let Some(interval) = data_domain.get_if_absolute_value() {
+        if !interval.is_top() {
+            return true;
+        }
+    }
+    false
+}
+
 /// 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.
@@ -105,13 +146,32 @@ pub fn check_cwe(
     cwe_params: &serde_json::Value,
 ) -> (Vec<LogMessage>, Vec<CweWarning>) {
     let project = analysis_results.project;
+    let pointer_inference_results = analysis_results.pointer_inference.unwrap();
+
     let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
     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, jump, symbol) in get_callsites(sub, &symbol_map) {
             if block_contains_multiplication(block) {
+                let parms = match symbol.name.as_str() {
+                    "calloc" => {
+                        if calloc_parm_mul_is_top(
+                            pointer_inference_results,
+                            &jump.tid,
+                            vec![&symbol.parameters[0], &symbol.parameters[1]],
+                        ) {
                             cwe_warnings.push(generate_cwe_warning(&jump.tid, symbol));
+                        };
+                        vec![&symbol.parameters[0], &symbol.parameters[1]]
+                    }
+                    "realloc" => vec![&symbol.parameters[1]],
+                    _ => symbol.parameters.iter().collect(),
+                };
+
+                if contains_top_value(pointer_inference_results, &jump.tid, parms) {
+                    cwe_warnings.push(generate_cwe_warning(&jump.tid, symbol));
+                }
             }
         }
     }