Changed the check to work with taint-tracking.

src/config.json

@@ -81,6 +81,14 @@
       ]
     ]
   },
+  "CWE337": {
+    "sources": [
+      "time"
+    ],
+    "sinks": [
+      "srand"
+    ]
+  },
   "CWE367": {
     "pairs": [
       [

src/cwe_checker_lib/src/checkers/cwe_337.rs

@@ -1,12 +1,38 @@
-//! @cleanup
+//! This module implements a check for CWE-337: Predictable Seed in Pseudo-Random Number Generator (PRNG)
+//! 
+//! The use of predictable seeds significantly reduces the number of possible seeds that an attacker would need 
+//! to test in order to predict which random numbers will be generated by the PRNG.
+//! 
+//! See <https://cwe.mitre.org/data/definitions/337.html> for a detailed description.
+//!
+//! ## How the check works
+//!
+//! Using dataflow analysis we search for an execution path where the result of a time source, like `time`,
+//! is used as an argument to a PRNG seeding function, like `srand`.
+//!
+//! ### Symbols configurable in config.json
+//! 
+//! Both the sources of predictable seeds and the seeding functions can be configured using the `sources`
+//! and `sinks` respectively.
+//!
 
-use crate::analysis::graph::Node;
+use crate::analysis::graph::{Edge, Graph, Node};
 use crate::intermediate_representation::*;
 use crate::prelude::*;
 use crate::utils::log::{CweWarning, LogMessage};
-use crate::utils::symbol_utils::find_symbol;
 use crate::CweModule;
 
+use crate::abstract_domain::AbstractDomain;
+use crate::analysis::forward_interprocedural_fixpoint::create_computation;
+use crate::analysis::interprocedural_fixpoint_generic::NodeValue;
+
+use petgraph::visit::EdgeRef;
+use std::collections::BTreeMap;
+use std::collections::HashMap;
+
+use crate::checkers::cwe_476::state::*;
+use crate::checkers::cwe_476::taint::*;
+
 /// The module name and version
 pub static CWE_MODULE: CweModule = CweModule {
     name: "CWE337",
@@ -17,89 +43,209 @@ pub static CWE_MODULE: CweModule = CweModule {
 /// The configuration struct
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
 pub struct Config {
-    /// Sources of predictable pseudo-random numbers.
+    /// Sources of predictable seeds.
     sources: Vec<String>,
     /// Random number seeding functions.
     sinks: Vec<String>,
 }
 
-/// Check whether the given block calls the given TID.
-/// If yes, return the TID of the jump term that contains the call.
-fn blk_calls_tid(blk: &Term<Blk>, tid: &Tid) -> Option<Tid> {
-    for jmp in blk.term.jmps.iter() {
-        match &jmp.term {
-            Jmp::Call { target, .. } if target == tid => {
-                return Some(jmp.tid.clone());
-            }
-            _ => (),
-        }
-    }
-    None
-}
-
-/// Generate a CWE warning for a CWE hit.
-fn generate_cwe_warning(rng_sub: &Term<Sub>, rng_callsite: &Tid, seed_sub: &Term<Sub>, seed_callsite: &Tid) -> CweWarning {
-    CweWarning::new(
-        CWE_MODULE.name,
-        CWE_MODULE.version,
-        format!(
-            "Call of {} at {} leads immetiately into call of {} at {}.",
-            rng_sub.term.name, rng_callsite.address, seed_sub.term.name, seed_callsite.address, 
-        ))
-        .tids(vec![format!("{rng_callsite}"), format!("{seed_callsite}")])
-        .addresses(vec![rng_callsite.address.clone(), seed_callsite.address.clone()])
-        .symbols(vec![rng_sub.term.name.clone(), seed_sub.term.name.clone()])
-}
-
+/// This check checks if a return value of any of the sources (as determined by the config file)
+/// is used as a direct parameter of any of the sinks (as determined by the config file).
+/// Currently, this is only used to detect whether a call of `time` leads into a call of `srand`,
+/// only tracks registers and not memory loctions and stops at any call.
 pub fn check_cwe(
     analysis_results: &AnalysisResults,
-    _cwe_params: &serde_json::Value,
+    cwe_params: &serde_json::Value,
 ) -> (Vec<LogMessage>, Vec<CweWarning>) {
     let project = analysis_results.project;
     let graph = analysis_results.control_flow_graph;
-
-    // @hmm: probably do this in the future?
-    // let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
-    // let _sources = config.sources.iter().cloned().collect();
-    // let _sinks = config.sinks.iter().cloned().collect();
-
-    let srand_tid = match find_symbol(&project.program, "srand") {
-        Some((tid, _)) => tid.clone(),
-        None => return (Vec::new(), Vec::new()), // srand is never called by the program
-    };
-
-    let time_tid = match find_symbol(&project.program, "time") {
-        Some((tid, _)) => tid.clone(),
-        None => return (Vec::new(), Vec::new()), // time is never called by the program
-    };
-
-    let mut cwe_warnings = Vec::new();
-    for time_call_node in graph.node_indices() {
-        if let Node::BlkEnd(time_call_block, time_subprocedure) = graph[time_call_node] {
-            if let Some(time_callsite_tid) = blk_calls_tid(time_call_block, &time_tid) {
-
-                if graph.neighbors(time_call_node).count() > 1 {
-                    panic!("Malformed Control flow graph: More than one edge for extern function call")
-                }
-
-
-                let time_return_node = graph.neighbors(time_call_node).next().unwrap();
-
-                if let Node::BlkStart(time_return_block, srand_subprocedure) = graph[time_return_node] { // @cleanup: Is this always true?
-                    if let Some(srand_callsite_tid) = blk_calls_tid(time_return_block, &srand_tid) {
-                        cwe_warnings.push(generate_cwe_warning(time_subprocedure, &time_callsite_tid, srand_subprocedure, &srand_callsite_tid));
-                    }
+
+    let (cwe_sender, cwe_receiver) = crossbeam_channel::unbounded();
+
+    let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
+    let source_map = crate::utils::symbol_utils::get_symbol_map(project, &config.sources[..]);
+    let sink_map = crate::utils::symbol_utils::get_symbol_map(project, &config.sinks[..]);
+
+    for edge in graph.edge_references() {
+        if let Edge::ExternCallStub(jmp) = edge.weight() {
+            if let Jmp::Call { target, .. } = &jmp.term {
+                if let Some(symbol) = source_map.get(target) {
+                    let node = edge.target();
+
+                    let context = Context {
+                        control_flow_graph: graph,
+                        sink_map: &sink_map,
+                        cwe_collector: &cwe_sender,
+                        source_call: jmp,
+                        source_symbol: symbol,
+                    };
+
+                    let mut computation = create_computation(context, None);
+                    computation.set_node_value(node, NodeValue::Value(State::new(symbol, None)));
+                    computation.compute_with_max_steps(100); // FIXME: This number should be in the config.
                 }
             }
         }
     }
-
+
+    let mut cwe_warnings = BTreeMap::new();
+    for cwe in cwe_receiver.try_iter() {
+        match &cwe.addresses[..] {
+            [taint_source_address, ..] => cwe_warnings.insert(taint_source_address.clone(), cwe),
+            _ => panic!(),
+        };
+    }
+    let cwe_warnings = cwe_warnings.into_values().collect();
+
     (Vec::new(), cwe_warnings)
 }
 
+/// The Context struct for the forward_interprocedural_fixpoint algorithm.
+pub struct Context<'a> {
+    /// The underlying control flow graph for the algorithm.
+    control_flow_graph: &'a Graph<'a>,
+    /// A map of symbols to use as sinks for the algorithm.
+    sink_map: &'a HashMap<Tid, &'a ExternSymbol>,
+    /// The call whose return values are the sources for taint for the analysis.
+    source_call: &'a Term<Jmp>,
+    /// The symbol associated with the `source_call`.
+    source_symbol: &'a ExternSymbol,
+    /// A channel where found CWE hits can be sent to.
+    cwe_collector: &'a crossbeam_channel::Sender<CweWarning>,
+}
 
+impl<'a> Context<'a> {
+    fn generate_cwe_warning(&self, sink_call: &Term<Jmp>, sink_symbol: &ExternSymbol) {
+        let cwe_warning = CweWarning::new(
+            CWE_MODULE.name,
+            CWE_MODULE.version,
+            format!(
+                "Return value of {} at {} leads into call of {} at {}.",
+                self.source_symbol.name,
+                self.source_call.tid.address,
+                sink_symbol.name,
+                sink_call.tid.address,
+            ),
+        )
+        .tids(vec![
+            format!("{}", self.source_call.tid),
+            format!("{}", sink_call.tid),
+        ])
+        .addresses(vec![
+            self.source_call.tid.address.clone(),
+            sink_call.tid.address.clone(),
+        ])
+        .symbols(vec![
+            self.source_symbol.name.clone(),
+            sink_symbol.name.clone(),
+        ]);
+        let _ = self.cwe_collector.send(cwe_warning);
+    }
+}
 
+impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Context<'a> {
+    type Value = State;
 
+    /// Provide access to the control flow graph.
+    fn get_graph(&self) -> &Graph<'a> {
+        self.control_flow_graph
+    }
 
+    /// Just forward the value merging to the [`AbstractDomain`].
+    fn merge(&self, value_1: &Self::Value, value_2: &Self::Value) -> Self::Value {
+        value_1.merge(value_2)
+    }
 
+    /// Keep track of register taint trough Defs.
+    /// Currently, we never taint memory.
+    fn update_def(&self, state: &State, def: &Term<Def>) -> Option<Self::Value> {
+        if state.is_empty() {
+            return None;
+        }
+
+        let mut new_state = state.clone();
+        match &def.term {
+            Def::Assign { var, value } => {
+                new_state.set_register_taint(var, state.eval(value));
+            }
+            Def::Load { var, .. } => {
+                // @cleanup: CWE476 uses pointer_inference here to load taint from memory.
+                new_state.set_register_taint(var, Taint::Top(var.size));
+            }
+            Def::Store { .. } => {}
+        }
+
+        Some(new_state)
+    }
+
+    /// We don't care if there was some condition on the random value.
+    fn update_jump(
+        &self,
+        state: &State,
+        _jump: &Term<Jmp>,
+        _untaken_conditional: Option<&Term<Jmp>>,
+        _target: &Term<Blk>,
+    ) -> Option<Self::Value> {
+        Some(state.clone())
+    }
+
+    /// For now we stop the search on any sort of call.
+    fn update_call(
+        &self,
+        _value: &Self::Value,
+        _call: &Term<Jmp>,
+        _target: &Node,
+        _calling_convention: &Option<String>,
+    ) -> Option<Self::Value> {
+        None
+    }
+
+    /// For now we stop the search on any sort of call.
+    fn update_return(
+        &self,
+        _value: Option<&Self::Value>,
+        _value_before_call: Option<&Self::Value>,
+        _call_term: &Term<Jmp>,
+        _return_term: &Term<Jmp>,
+        _calling_convention: &Option<String>,
+    ) -> Option<Self::Value> {
+        None
+    }
+
+    /// For now we stop the search on any sort of call.
+    /// But report a cwe warning, if we encountered a call to one of the sink symbols.
+    fn update_call_stub(&self, state: &State, call: &Term<Jmp>) -> Option<Self::Value> {
+        if state.is_empty() {
+            return None;
+        }
+
+        match &call.term {
+            Jmp::Call { target, .. } => {
+                if let Some(sink_symbol) = self.sink_map.get(target) {
+                    for parameter in sink_symbol.parameters.iter() {
+                        if let Arg::Register { expr, .. } = parameter {
+                            if state.eval(expr).is_tainted() {
+                                self.generate_cwe_warning(call, sink_symbol);
+                            }
+                        }
+                    }
+                }
+            }
+            Jmp::CallInd { .. } => {}
+            _ => panic!("Malformed control flow graph encountered."),
+        }
+
+        None
+    }
 
+    /// I have no idea what this is but I copied the implementation (as almost everything else) from CWE476.
+    fn specialize_conditional(
+        &self,
+        state: &State,
+        _condition: &Expression,
+        _block_before_condition: &Term<Blk>,
+        _is_true: bool,
+    ) -> Option<Self::Value> {
+        Some(state.clone())
+    }
+}

src/cwe_checker_lib/src/checkers/cwe_476.rs

@@ -47,10 +47,10 @@ use crate::CweModule;
 use petgraph::visit::EdgeRef;
 use std::collections::BTreeMap;
 
-mod state;
+pub mod state;
 use state::*;
 
-mod taint;
+pub mod taint;
 pub use taint::*;
 
 mod context;