Reimplement CWE 243 check (#119)

caller/src/main.rs

@@ -141,9 +141,10 @@ fn run_with_ghidra(args: CmdlineArgs) {
     let mut all_logs = project.normalize();
     let mut analysis_results = AnalysisResults::new(&project);
 
+    let modules_depending_on_pointer_inference = vec!["CWE243", "CWE367", "CWE476", "Memory"];
     let pointer_inference_results = if modules
         .iter()
-        .any(|module| module.name == "CWE476" || module.name == "Memory" || module.name == "CWE367")
+        .any(|module| modules_depending_on_pointer_inference.contains(&module.name))
     {
         Some(analysis_results.compute_pointer_inference(&config["Memory"]))
     } else {

cwe_checker_rs/src/checkers.rs

 pub mod cwe_190;
+pub mod cwe_243;
 pub mod cwe_332;
 pub mod cwe_367;
 pub mod cwe_426;

cwe_checker_rs/src/checkers/cwe_243.rs

+//! This module implements a check for CWE-243: Creation of chroot Jail Without Changing Working Directory.
+//!
+//! Creating a chroot Jail without changing the working directory afterwards does
+//! not prevent access to files outside of the jail.
+//!
+//! See <https://cwe.mitre.org/data/definitions/243.html> for detailed a description.
+//!
+//! ## How the check works
+//!
+//! According to <http://www.unixwiz.net/techtips/chroot-practices.html>, there are
+//! several ways to achieve the safe creation of a chroot jail.
+//! One can either call chdir after chroot
+//! or, if chdir is called before chroot, drop priviledges after the chroot call.
+//! The functions used to drop priviledges are configurable in config.json.
+//! We check whether each function that calls
+//! chroot is using one of these safe call sequences to create the chroot jail.
+//! If not, a warning is emitted.
+//!
+//! ## False Positives
+//!
+//! None known.
+//!
+//! ## False Negatives
+//!
+//! We do not check whether the parameters to chdir, chroot and the priviledge dropping functions
+//! are suitable to create a safe chroot jail.
+
+use crate::analysis::graph::Node;
+use crate::intermediate_representation::*;
+use crate::prelude::*;
+use crate::utils::graph_utils::is_sink_call_reachable_from_source_call;
+use crate::utils::log::{CweWarning, LogMessage};
+use crate::utils::symbol_utils::find_symbol;
+use crate::CweModule;
+
+pub static CWE_MODULE: CweModule = CweModule {
+    name: "CWE243",
+    version: "0.2",
+    run: check_cwe,
+};
+
+/// The configuration struct contains the list of functions
+/// that are assumed to be used to correctly drop priviledges after a `chroot` call.
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
+pub struct Config {
+    priviledge_dropping_functions: 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
+}
+
+/// Check whether the given `sub` calls both the `chdir_tid`
+/// and at least one of the `priviledge_dropping_tids`.
+/// If yes, return true.
+fn sub_calls_chdir_and_priviledge_dropping_func(
+    sub: &Term<Sub>,
+    chdir_tid: &Tid,
+    priviledge_dropping_tids: &[Tid],
+) -> bool {
+    let mut is_chdir_called = false;
+    for blk in sub.term.blocks.iter() {
+        if blk_calls_tid(blk, chdir_tid).is_some() {
+            is_chdir_called = true;
+            break;
+        }
+    }
+    if !is_chdir_called {
+        return false;
+    }
+    for blk in sub.term.blocks.iter() {
+        if priviledge_dropping_tids
+            .iter()
+            .any(|tid| blk_calls_tid(blk, tid).is_some())
+        {
+            return true;
+        }
+    }
+    false
+}
+
+/// Generate a CWE warning for a CWE hit.
+fn generate_cwe_warning(sub: &Term<Sub>, callsite: &Tid) -> CweWarning {
+    CweWarning::new(
+        CWE_MODULE.name,
+        CWE_MODULE.version,
+        format!(
+            "(The program utilizes chroot without dropping privileges and/or changing the directory) at {} ({})",
+            callsite.address, sub.term.name
+        ))
+        .tids(vec![format!("{}", callsite)])
+        .addresses(vec![callsite.address.clone()])
+        .symbols(vec![sub.term.name.clone()])
+}
+
+/// Run the check.
+///
+/// For each call to `chroot` we check
+/// - that it is either followed by a call to `chdir` in the same function
+/// - or that the same function contains calls to `chdir`
+/// and a call to a function that can be used to drop priviledges.
+///
+/// If both are false, we assume that the chroot-jail is insecure and report a CWE hit.
+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.pointer_inference.unwrap().get_graph();
+
+    let config: Config = serde_json::from_value(cwe_params.clone()).unwrap();
+    let priviledge_dropping_tids: Vec<Tid> = config
+        .priviledge_dropping_functions
+        .into_iter()
+        .filter_map(|func_name| {
+            if let Some((tid, _)) = find_symbol(&project.program, &func_name) {
+                Some(tid.clone())
+            } else {
+                None
+            }
+        })
+        .collect();
+
+    let chroot_tid = match find_symbol(&project.program, "chroot") {
+        Some((tid, _)) => tid.clone(),
+        None => return (Vec::new(), Vec::new()), // chroot is never called by the program
+    };
+
+    let mut cwe_warnings = Vec::new();
+    for node in graph.node_indices() {
+        if let Node::BlkEnd(blk, sub) = graph[node] {
+            if let Some(callsite_tid) = blk_calls_tid(blk, &chroot_tid) {
+                if let Some(chdir_tid) =
+                    find_symbol(&project.program, "chdir").map(|(tid, _)| tid.clone())
+                {
+                    // If chdir is called after chroot, we assume a secure chroot jail.
+                    if is_sink_call_reachable_from_source_call(graph, node, &chroot_tid, &chdir_tid)
+                        .is_none()
+                    {
+                        // If chdir is not called after chroot, it has to be called before it.
+                        // Additionally priviledges must be dropped to secure the chroot jail in this case.
+                        if !sub_calls_chdir_and_priviledge_dropping_func(
+                            sub,
+                            &chdir_tid,
+                            &priviledge_dropping_tids[..],
+                        ) {
+                            cwe_warnings.push(generate_cwe_warning(sub, &callsite_tid));
+                        }
+                    }
+                } else {
+                    // There is no chdir symbol, so the chroot jail cannot be secured.
+                    cwe_warnings.push(generate_cwe_warning(sub, &callsite_tid));
+                }
+            }
+        }
+    }
+
+    (Vec::new(), cwe_warnings)
+}

cwe_checker_rs/src/checkers/cwe_367.rs

@@ -22,14 +22,14 @@
 //! - If the check-call and the use-call happen in different functions it will not
 //!   be found by the check.
 
-use crate::analysis::graph::{Edge, Graph, Node};
+use crate::analysis::graph::{Edge, Node};
 use crate::intermediate_representation::Jmp;
 use crate::prelude::*;
+use crate::utils::graph_utils::is_sink_call_reachable_from_source_call;
 use crate::utils::log::{CweWarning, LogMessage};
 use crate::CweModule;
-use petgraph::graph::NodeIndex;
 use petgraph::visit::EdgeRef;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 
 pub static CWE_MODULE: CweModule = CweModule {
     name: "CWE367",
@@ -45,56 +45,6 @@ struct Config {
     pairs: Vec<(String, String)>,
 }
 
-/// Check whether a call to the `sink_symbol` is reachable from the `source_node`
-/// through a path of intraprocedural edges in the control flow graph.
-///
-/// A simple depth-first-search on the graph is used to find such a path.
-fn is_reachable(
-    graph: &Graph,
-    source_node: NodeIndex,
-    source_symbol: &Tid,
-    sink_symbol: &Tid,
-) -> Option<(Tid, Tid)> {
-    let mut visited_nodes = HashSet::new();
-    visited_nodes.insert(source_node);
-    let mut worklist = vec![source_node];
-
-    while let Some(node) = worklist.pop() {
-        for edge in graph.edges(node) {
-            if let Edge::ExternCallStub(jmp) = edge.weight() {
-                if let Jmp::Call { target, .. } = &jmp.term {
-                    if target == sink_symbol {
-                        // We found a CWE hit
-                        let source_tid = graph[source_node].get_block().tid.clone();
-                        return Some((source_tid, jmp.tid.clone()));
-                    } else if target == source_symbol {
-                        // Do not search past another source call,
-                        // since subsequent sink calls probably belong to the new source.
-                        continue;
-                    }
-                }
-            }
-            // Add the target node to the worklist if it was not already visited
-            // and as long as the edge does not leave the function.
-            match edge.weight() {
-                Edge::Block
-                | Edge::CRCallStub
-                | Edge::CallCombine(_)
-                | Edge::ReturnCombine(_)
-                | Edge::Jump(_, _)
-                | Edge::ExternCallStub(_) => {
-                    if visited_nodes.get(&edge.target()).is_none() {
-                        visited_nodes.insert(edge.target());
-                        worklist.push(edge.target())
-                    }
-                }
-                Edge::Call(_) | Edge::CRReturnStub => (), // These edges would leave the function control flow graph.
-            }
-        }
-    }
-    None
-}
-
 /// Generate a CWE warning for a found CWE hit.
 fn generate_cwe_warning(
     source: &str,
@@ -142,9 +92,13 @@ pub fn check_cwe(
                 if let Edge::ExternCallStub(jmp) = edge.weight() {
                     if let Jmp::Call { target, .. } = &jmp.term {
                         if target == source_tid {
-                            if let Some((source_callsite, sink_callsite)) =
-                                is_reachable(graph, edge.target(), target, sink_tid)
-                            {
+                            if let Some(sink_callsite) = is_sink_call_reachable_from_source_call(
+                                graph,
+                                edge.target(),
+                                target,
+                                sink_tid,
+                            ) {
+                                let source_callsite = graph[edge.target()].get_block().tid.clone();
                                 let sub_name = match graph[edge.target()] {
                                     Node::BlkStart(_blk, sub) => sub.term.name.as_str(),
                                     _ => panic!("Malformed control flow graph."),

cwe_checker_rs/src/checkers/cwe_782.rs

 /*!
 This module implements a check for CWE-782: Exposed IOCTL with Insufficient Access Control.
+
 See <https://cwe.mitre.org/data/definitions/782.html> for a detailed description.
 
 How the check works:

cwe_checker_rs/src/lib.rs

@@ -55,6 +55,7 @@ impl std::fmt::Display for CweModule {
 pub fn get_modules() -> Vec<&'static CweModule> {
     vec![
         &crate::checkers::cwe_190::CWE_MODULE,
+        &crate::checkers::cwe_243::CWE_MODULE,
         &crate::checkers::cwe_332::CWE_MODULE,
         &crate::checkers::cwe_367::CWE_MODULE,
         &crate::checkers::cwe_426::CWE_MODULE,

cwe_checker_rs/src/utils/graph_utils.rs

+use crate::analysis::graph::*;
+use crate::intermediate_representation::Jmp;
+use crate::prelude::*;
+use petgraph::graph::NodeIndex;
+use petgraph::visit::EdgeRef;
+use std::collections::HashSet;
+
+/// Check whether a call to the `sink_symbol` is reachable from the given `source_node`
+/// through a path of intraprocedural edges in the control flow graph.
+///
+/// A simple depth-first-search on the graph is used to find such a path.
+/// We do not search past subsequent calls to the `source_symbol`
+/// since we assume that sink calls after that belong to the new call to the source symbol and not the original one.
+///
+/// If a sink is found, the `Tid` of the jump term calling the sink is returned.
+pub fn is_sink_call_reachable_from_source_call(
+    graph: &Graph,
+    source_node: NodeIndex,
+    source_symbol: &Tid,
+    sink_symbol: &Tid,
+) -> Option<Tid> {
+    let mut visited_nodes = HashSet::new();
+    visited_nodes.insert(source_node);
+    let mut worklist = vec![source_node];
+
+    while let Some(node) = worklist.pop() {
+        for edge in graph.edges(node) {
+            if let Edge::ExternCallStub(jmp) = edge.weight() {
+                if let Jmp::Call { target, .. } = &jmp.term {
+                    if target == sink_symbol {
+                        // We found a call to the sink
+                        return Some(jmp.tid.clone());
+                    } else if target == source_symbol {
+                        // Do not search past another source call,
+                        // since subsequent sink calls probably belong to the new source.
+                        continue;
+                    }
+                }
+            }
+            // Add the target node to the worklist if it was not already visited
+            // and as long as the edge does not leave the function.
+            match edge.weight() {
+                Edge::Block
+                | Edge::CRCallStub
+                | Edge::CallCombine(_)
+                | Edge::ReturnCombine(_)
+                | Edge::Jump(_, _)
+                | Edge::ExternCallStub(_) => {
+                    if visited_nodes.get(&edge.target()).is_none() {
+                        visited_nodes.insert(edge.target());
+                        worklist.push(edge.target())
+                    }
+                }
+                Edge::Call(_) | Edge::CRReturnStub => (), // These edges would leave the function control flow graph.
+            }
+        }
+    }
+    None
+}

cwe_checker_rs/src/utils/mod.rs

+pub mod graph_utils;
 pub mod log;
 pub mod symbol_utils;
 

src/config.json

@@ -37,6 +37,12 @@
         "chroot",
         "setuid"
       ]
+    ],
+    "priviledge_dropping_functions": [
+      "setresuid",
+      "seteuid",
+      "setreuid",
+      "setuid"
     ]
   },
   "CWE248": {

test/artificial_samples/cwe_243.c

@@ -10,6 +10,52 @@ void chroot_fail(){
   }
 }
 
+// these are safe according to http://www.unixwiz.net/techtips/chroot-practices.html
+void chroot_safe1(){
+  chdir("/tmp");
+  if (chroot("/tmp") != 0) {
+    perror("chroot /tmp");
+  }
+  setuid(1077);
+}
+
+void chroot_safe2(){
+  chdir("/tmp");
+  if (chroot("/tmp") != 0) {
+    perror("chroot /tmp");
+  }
+  setresuid(1077, 1077, 1077);
+}
+
+void chroot_safe3(){
+  chdir("/tmp");
+  if (chroot("/tmp") != 0) {
+    perror("chroot /tmp");
+  }
+  setreuid(1077, 44);
+}
+
+void chroot_safe4(){
+  chdir("/tmp");
+  if (chroot("/tmp") != 0) {
+    perror("chroot /tmp");
+  }
+  seteuid(1077);
+}
+
+void chroot_safe5(){
+  if (chroot("/tmp") != 0) {
+    perror("chroot /tmp");
+  }
+  chdir("/");
+}
+
 int main(void) {
   chroot_fail();
+
+  chroot_safe1();
+  chroot_safe2();
+  chroot_safe3();
+  chroot_safe4();
+  chroot_safe5();
 }

test/src/lib.rs

@@ -200,6 +200,27 @@ mod tests {
 
     #[test]
     #[ignore]
+    fn cwe_243() {
+        let mut error_log = Vec::new();
+        let mut tests = linux_test_cases("cwe_243", "CWE243");
+
+        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.
+
+        for test_case in tests {
+            let num_expected_occurences = 1;
+            if let Err(error) = test_case.run_test("[CWE243]", 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_332() {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_332", "CWE332");