From 9126ebf62b8d051a9b1c3e18223d562f4b48bf1e Mon Sep 17 00:00:00 2001 From: Pascal Beyer Date: Fri, 9 Feb 2024 14:03:10 +0100 Subject: [PATCH] Changed the check to work with taint-tracking. --- src/config.json | 8 + src/cwe_checker_lib/src/checkers/cwe_337.rs | 278 +++++++++++++++----- src/cwe_checker_lib/src/checkers/cwe_476.rs | 4 +- 3 files changed, 222 insertions(+), 68 deletions(-) diff --git a/src/config.json b/src/config.json index 1b4590b86..fd7cd6766 100644 --- a/src/config.json +++ b/src/config.json @@ -81,6 +81,14 @@ ] ] }, + "CWE337": { + "sources": [ + "time" + ], + "sinks": [ + "srand" + ] + }, "CWE367": { "pairs": [ [ diff --git a/src/cwe_checker_lib/src/checkers/cwe_337.rs b/src/cwe_checker_lib/src/checkers/cwe_337.rs index 5e4533537..af140a0cf 100644 --- a/src/cwe_checker_lib/src/checkers/cwe_337.rs +++ b/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 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, /// Random number seeding functions. sinks: Vec, } -/// 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, tid: &Tid) -> Option { - 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, rng_callsite: &Tid, seed_sub: &Term, 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, Vec) { 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, + /// The call whose return values are the sources for taint for the analysis. + source_call: &'a Term, + /// 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, +} +impl<'a> Context<'a> { + fn generate_cwe_warning(&self, sink_call: &Term, 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) -> Option { + 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, + _untaken_conditional: Option<&Term>, + _target: &Term, + ) -> Option { + Some(state.clone()) + } + + /// For now we stop the search on any sort of call. + fn update_call( + &self, + _value: &Self::Value, + _call: &Term, + _target: &Node, + _calling_convention: &Option, + ) -> Option { + 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, + _return_term: &Term, + _calling_convention: &Option, + ) -> Option { + 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) -> Option { + 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, + _is_true: bool, + ) -> Option { + Some(state.clone()) + } +} diff --git a/src/cwe_checker_lib/src/checkers/cwe_476.rs b/src/cwe_checker_lib/src/checkers/cwe_476.rs index d953526a2..0423c772c 100644 --- a/src/cwe_checker_lib/src/checkers/cwe_476.rs +++ b/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;