WIP: Update CWE 476

src/config.json

@@ -288,11 +288,8 @@
  "CWE476": {
  "_comment": "any function that possibly returns a NULL value.",
  "_comment1": "included functions of the following libs: stdlib.h, locale.h, stdio.h, cstring.h, wchar.h",
- "parameters": [
- "strict_call_policy=true",
- "strict_memory_policy=false",
- "max_steps=100"
- ],
+ "strict_call_policy": false,
+ "max_steps": 100,
  "symbols": [
  "malloc",
  "calloc",

src/cwe_checker_lib/src/abstract_domain/interval.rs

@@ -743,6 +743,13 @@ impl From<Bitvector> for IntervalDomain {
  }
 }
 
+impl From<i64> for IntervalDomain {
+ /// Create a new interval containing only `value`.
+ fn from(value: i64) -> Self {
+ Self::new(value.into(), value.into())
+ }
+}
+
 impl TryToBitvec for IntervalDomain {
  /// If the domain represents an interval of length one, return the contained value.
  fn try_to_bitvec(&self) -> Result<Bitvector, Error> {

src/cwe_checker_lib/src/analysis/function_signature/mod.rs

@@ -45,6 +45,7 @@ use crate::analysis::graph::*;
 use crate::analysis::interprocedural_fixpoint_generic::NodeValue;
 use crate::intermediate_representation::*;
 use crate::prelude::*;
+use crate::utils::debug::ToJsonCompact;
 use crate::utils::log::LogMessage;
 use std::collections::BTreeMap;
 
@@ -380,10 +381,10 @@ impl Default for FunctionSignature {
  }
 }
 
-impl FunctionSignature {
- /// Generate a compact JSON-representation of the function signature for pretty printing.
- #[allow(dead_code)]
- pub fn to_json_compact(&self) -> serde_json::Value {
+impl ToJsonCompact for FunctionSignature {
+ /// Generate a compact JSON-representation of the function signature for
+ /// pretty printing.
+ fn to_json_compact(&self) -> serde_json::Value {
  let mut json_map = serde_json::Map::new();
  let mut param_map = serde_json::Map::new();
  for (param, pattern) in self.parameters.iter() {

src/cwe_checker_lib/src/analysis/taint/mod.rs

@@ -201,21 +201,55 @@ pub trait TaintAnalysis<'a>: HasCfg<'a> + HasVsaResult<PiData> + AsRef<Project>
  /// site!); thus, return the empty state if you want to keep the analysis
  /// in the caller going.
  ///
- /// [`update_return`]: TaintAnalysis::update_return
- /// [`merged`]: State::merge
- ///
  /// # Default
  ///
- /// Returns an empty state, i.e., information is propagated through the call
- /// but the analysis stays intraprocedural.
+ /// Reduces register taint to include only the return registers of the
+ /// used calling convention that are tainted.
+ ///
+ /// Memory taint is returned as-is since [`update_return`] will make sure that
+ /// objects that can not be reached from the caller context are removed.
+ ///
+ /// [`merged`]: State::merge
+ /// [`update_return`]: TaintAnalysis::update_return
  fn update_return_callee(
  &self,
- _state: &State,
+ state: &State,
  _call_term: &Term<Jmp>,
  _return_term: &Term<Jmp>,
- _calling_convention: &Option<String>,
+ calling_convention: &Option<String>,
  ) -> Option<State> {
- Some(State::new_empty())
+ let (mut propagated_register_taint, memory_taint) = state.clone().into_mem_reg_taint();
+
+ if let Some(calling_convention) = <Self as AsRef<Project>>::as_ref(self)
+ .get_specific_calling_convention(calling_convention)
+ {
+ let return_registers = calling_convention.get_all_return_register();
+
+ // If there are tainted return registers we propagate the taint to
+ // the caller, which makes them responsible for handling it.
+ propagated_register_taint
+ .retain(|reg, taint| return_registers.contains(&reg) && taint.is_tainted());
+ } else {
+ // We might have tainted registers but we do not know the calling
+ // convention. Here we simply return the complete register taint
+ // of the callee to the caller. This heuristic should in practice
+ // hopefully be a good approximation to the real calling convention:
+ //
+ // - It is an over approximation so return registers will be
+ // propagated correctly.
+ // - There is a chance that callee-saved registers have been
+ // overwritten with their saved values by the function epilog and
+ // are thus not tainted.
+ // - There is a chance that caller saved registers will be restored
+ // by the caller such that the taint is immediatly eliminated and
+ // we catch cases where the called function has ignored tainted
+ // values.
+ };
+
+ Some(State::from_mem_reg_taint(
+ propagated_register_taint,
+ memory_taint,
+ ))
  }
 
  /// Corresponds to returns from calls to other functions within the program.

src/cwe_checker_lib/src/analysis/taint/state.rs

@@ -256,7 +256,7 @@ impl State {
  ///
  /// Returns true iff the value at any of the exact memory locations that the
  /// pointer may point to is tainted.
- pub fn check_if_address_points_to_taint(&self, address: PiData) -> bool {
+ pub fn check_if_address_points_to_taint(&self, address: &PiData) -> bool {
  address
  .get_relative_values()
  .iter()
@@ -294,7 +294,7 @@ impl State {
  POINTER_TAINT
  && vsa_result
  .eval_parameter_location_at_call(jmp_tid, &AbstractLocation::Register(register.clone()))
- .is_some_and(|register_value| self.check_if_address_points_to_taint(register_value))
+ .is_some_and(|register_value| self.check_if_address_points_to_taint(&register_value))
  )
  })
  }
@@ -367,6 +367,36 @@ impl State {
  }
  }
 
+ /// Check if the given abstract location may contain a tainted value.
+ pub fn check_abstract_location_for_taint(
+ &self,
+ vsa_result: &impl VsaResult<ValueDomain = PiData>,
+ call_tid: &Tid,
+ abstract_location: &AbstractLocation,
+ ) -> bool {
+ use crate::abstract_domain::AbstractLocation::*;
+ match abstract_location {
+ GlobalAddress { .. } => false,
+ Register(reg) => self.get_register_taint(reg).is_tainted(),
+ _ => {
+ // TODO: Check if it is problematic that we hard code a pointer
+ // size of 8 bytes. Maybe there is a more elegant way.
+ let (parent_location, offset) = abstract_location
+ .get_parent_location(ByteSize::new(8))
+ .unwrap();
+
+ let Some(parent_value) =
+ vsa_result.eval_parameter_location_at_call(call_tid, &parent_location)
+ else {
+ return false;
+ };
+ let parent_value = parent_value.add_offset(&offset.into());
+
+ self.check_if_address_points_to_taint(&parent_value)
+ }
+ }
+ }
+
  /// Remove the taint from all registers not contained in the callee-saved
  /// register list of the given calling convention.
  pub fn remove_non_callee_saved_taint(&mut self, calling_conv: &CallingConvention) {
@@ -405,7 +435,7 @@ impl State {
  ||
  // Check if value in parameter register points to taint.
  (POINTER_TAINT && vsa_result.eval_at_jmp(call_tid, expr).is_some_and(|register_value| {
- self.check_if_address_points_to_taint(register_value)
+ self.check_if_address_points_to_taint(&register_value)
  }))
  }
  Arg::Stack { address, size, .. } => {
@@ -417,7 +447,7 @@ impl State {
  ||
  // Check if stack-based argument points to taint.
  (POINTER_TAINT && vsa_result.eval_parameter_arg_at_call(call_tid, parameter).is_some_and(|stack_value| {
- self.check_if_address_points_to_taint(stack_value)
+ self.check_if_address_points_to_taint(&stack_value)
  }))
  },
  }

src/cwe_checker_lib/src/checkers/cwe_476.rs

@@ -39,7 +39,7 @@
 
 use crate::analysis::forward_interprocedural_fixpoint::create_computation;
 use crate::analysis::forward_interprocedural_fixpoint::Context as _;
-use crate::analysis::graph::{Edge, Node};
+use crate::analysis::graph::Edge;
 use crate::analysis::interprocedural_fixpoint_generic::NodeValue;
 use crate::analysis::taint::state::State as TaState;
 use crate::intermediate_representation::*;
@@ -64,12 +64,20 @@ pub static CWE_MODULE: CweModule = CweModule {
 };
 
 /// The configuration struct.
+///
+/// These values are configurable via the `config.json` and `lkm_config.json`
+/// files.
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
 pub struct Config {
+ /// If `true`, passing a tainted value, or a pointer to an object that
+ /// contains taint, as an argument to a function call is more likely to
+ /// cause a warning.
+ strict_call_policy: bool,
+ /// Threshold for the fixpoint computation.
+ max_steps: u64,
  /// The names of symbols for which the analysis should check whether the
  /// return values are checked for being a NULL pointer by the analysed
- /// binary. This list is configurable via the `config.json` configuration
- /// file.
+ /// binary.
  symbols: Vec<String>,
 }
 
@@ -89,7 +97,13 @@ pub fn check_cwe(
 
  let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
  let symbol_map = symbol_utils::get_symbol_map(project, &config.symbols[..]);
- let general_context = Context::new(project, pi_result, cwe_sender);
+ let general_context = Context::new(
+ &config,
+ project,
+ pi_result,
+ analysis_results.function_signatures.unwrap(),
+ cwe_sender,
+ );
 
  for edge in general_context.get_graph().edge_references() {
  let Edge::ExternCallStub(jmp) = edge.weight() else {
@@ -102,19 +116,16 @@ pub fn check_cwe(
  continue;
  };
  let return_node = edge.target();
- let Node::BlkStart(.., current_sub) = general_context.get_graph()[return_node] else {
- panic!("Malformed control flow graph.");
- };
 
  let mut context = general_context.clone();
- context.set_taint_source(jmp, current_sub);
+ context.set_taint_source(jmp);
 
  let mut computation = create_computation(context, None);
  computation.set_node_value(
  return_node,
  NodeValue::Value(TaState::new_return(symbol, pi_result, return_node)),
  );
- computation.compute_with_max_steps(100);
+ computation.compute_with_max_steps(config.max_steps);
  }
 
  let mut cwe_warnings = BTreeMap::new();