Implement check for CWE-337 (#439)

README.md

@@ -103,6 +103,7 @@ So far the following analyses are implemented:
 -   [CWE-215](https://cwe.mitre.org/data/definitions/215.html): Information Exposure Through Debug Information
 -   [CWE-243](https://cwe.mitre.org/data/definitions/243.html): Creation of chroot Jail Without Changing Working Directory
 -   [CWE-332](https://cwe.mitre.org/data/definitions/332.html): Insufficient Entropy in PRNG
+-   [CWE-337](https://cwe.mitre.org/data/definitions/337.html): Predictable Seed in Pseudo-Random Number Generator (PRNG)
 -   [CWE-367](https://cwe.mitre.org/data/definitions/367.html): Time-of-check Time-of-use (TOCTOU) Race Condition
 -   [CWE-416](https://cwe.mitre.org/data/definitions/416.html): Use After Free and its variant [CWE-415](https://cwe.mitre.org/data/definitions/415.html): Double Free
 -   [CWE-426](https://cwe.mitre.org/data/definitions/426.html): Untrusted Search Path

src/config.json

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

src/cwe_checker_lib/src/checkers.rs

@@ -11,6 +11,7 @@ pub mod cwe_190;
 pub mod cwe_215;
 pub mod cwe_243;
 pub mod cwe_332;
+pub mod cwe_337;
 pub mod cwe_367;
 pub mod cwe_416;
 pub mod cwe_426;

src/cwe_checker_lib/src/checkers/cwe_337.rs

@@ -0,0 +1,234 @@
+//! 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 `seeding_functions` respectively.
+
+use crate::analysis::graph::{Edge, Graph, Node};
+use crate::intermediate_representation::*;
+use crate::prelude::*;
+use crate::utils::log::{CweWarning, LogMessage};
+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",
+    version: "0.1",
+    run: check_cwe,
+};
+
+/// The configuration struct
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
+pub struct Config {
+    /// Sources of predictable seeds.
+    sources: Vec<String>,
+    /// Random number seeding functions.
+    seeding_functions: Vec<String>,
+}
+
+/// 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 locations and stops at any call.
+pub fn check_cwe(
+    analysis_results: &AnalysisResults,
+    cwe_params: &serde_json::Value,
+) -> (Vec<LogMessage>, Vec<CweWarning>) {
+    let project = analysis_results.project;
+    let graph = analysis_results.control_flow_graph;
+
+    let (cwe_sender, cwe_receiver) = crossbeam_channel::unbounded();
+
+    let config: Config = serde_json::from_value(cwe_params.clone())
+        .expect("Invalid configuration inside config.json for CWE337.");
+    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.seeding_functions[..]);
+
+    if source_map.is_empty() || sink_map.is_empty() {
+        return (Vec::new(), Vec::new());
+    }
+
+    let context = Context {
+        control_flow_graph: graph,
+        sink_map: &sink_map,
+        cwe_collector: &cwe_sender,
+    };
+    let mut computation = create_computation(context, None);
+
+    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();
+                    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() {
+        cwe_warnings.insert(cwe.addresses[0].clone(), cwe);
+    }
+    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>,
+    /// 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!(
+                "RNG seed function {} at {} is seeded with predictable seed source.",
+                sink_symbol.name, sink_call.tid.address,
+            ),
+        )
+        .tids(vec![format!("{}", sink_call.tid)])
+        .addresses(vec![sink_call.tid.address.clone()])
+        .symbols(vec![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 through 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, .. } => {
+                // FIXME: 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
+    }
+
+    /// We don't care if there was some condition on the random value.
+    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,12 @@ use crate::CweModule;
 use petgraph::visit::EdgeRef;
 use std::collections::BTreeMap;
 
-mod state;
+/// Module for the taint tracking state. Reused by the check for CWE-337, hence public.
+pub mod state;
 use state::*;
 
-mod taint;
+/// Taint tracking module. Reused by the check for CWE-337, hence public.
+pub mod taint;
 pub use taint::*;
 
 mod context;

src/cwe_checker_lib/src/lib.rs

@@ -115,6 +115,7 @@ pub fn get_modules() -> Vec<&'static CweModule> {
         &crate::checkers::cwe_215::CWE_MODULE,
         &crate::checkers::cwe_243::CWE_MODULE,
         &crate::checkers::cwe_332::CWE_MODULE,
+        &crate::checkers::cwe_337::CWE_MODULE,
         &crate::checkers::cwe_367::CWE_MODULE,
         &crate::checkers::cwe_416::CWE_MODULE,
         &crate::checkers::cwe_426::CWE_MODULE,

test/artificial_samples/cwe_337.c

@@ -0,0 +1,10 @@
+
+#include <time.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+int main(int argc, char *argv[]){
+    srand(time(NULL));
+    return rand();
+}
+

test/src/lib.rs

@@ -394,6 +394,29 @@ mod tests {
         }
     }
 
+    #[test]
+    #[ignore]
+    fn cwe_337() {
+        let mut error_log = Vec::new();
+        let mut tests = all_test_cases("cwe_337", "CWE337");
+
+        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_architecture_skipped(&mut tests, "x86"); // x86 uses the stack for return values/arguments, the check is only register based.
+
+        for test_case in tests {
+            let num_expected_occurences = 1;
+            if let Err(error) = test_case.run_test("[CWE337]", num_expected_occurences) {
+                error_log.push((test_case.get_filepath(), error));
+            }
+        }
+        if !error_log.is_empty() {
+            print_errors(error_log);
+            panic!();
+        }
+    }
+
     #[test]
     #[ignore]
     fn cwe_367() {