reimplement CWE 560 check in Rust (#95)

cwe_checker_rs/src/analysis/pointer_inference/context/mod.rs

@@ -161,27 +161,6 @@ impl<'a> Context<'a> {
         }
     }
 
-    /// Evaluate the value of a parameter of an extern symbol for the given state.
-    fn eval_parameter_arg(&self, state: &State, parameter: &Arg) -> Result<Data, Error> {
-        match parameter {
-            Arg::Register(var) => state.eval(&Expression::Var(var.clone())),
-            Arg::Stack { offset, size } => state.load_value(
-                &Expression::BinOp {
-                    op: BinOpType::IntAdd,
-                    lhs: Box::new(Expression::Var(self.project.stack_pointer_register.clone())),
-                    rhs: Box::new(Expression::Const(
-                        Bitvector::from_i64(*offset)
-                            .into_truncate(apint::BitWidth::from(
-                                self.project.get_pointer_bytesize(),
-                            ))
-                            .unwrap(),
-                    )),
-                },
-                *size,
-            ),
-        }
-    }
-
     /// Mark the object that the parameter of a call is pointing to as freed.
     /// If the object may have been already freed, generate a CWE warning.
     /// This models the behaviour of `free` and similar functions.
@@ -194,7 +173,8 @@ impl<'a> Context<'a> {
     ) -> Option<State> {
         match extern_symbol.get_unique_parameter() {
             Ok(parameter) => {
-                let parameter_value = self.eval_parameter_arg(state, parameter);
+                let parameter_value =
+                    state.eval_parameter_arg(parameter, &self.project.stack_pointer_register);
                 match parameter_value {
                     Ok(memory_object_pointer) => {
                         if let Data::Pointer(pointer) = memory_object_pointer {
@@ -249,7 +229,7 @@ impl<'a> Context<'a> {
         extern_symbol: &ExternSymbol,
     ) {
         for parameter in extern_symbol.parameters.iter() {
-            match self.eval_parameter_arg(state, parameter) {
+            match state.eval_parameter_arg(parameter, &self.project.stack_pointer_register) {
                 Ok(value) => {
                     if state.memory.is_dangling_pointer(&value, true) {
                         let warning = CweWarning {
@@ -304,7 +284,9 @@ impl<'a> Context<'a> {
             }
         } else {
             for parameter in extern_symbol.parameters.iter() {
-                if let Ok(data) = self.eval_parameter_arg(state, parameter) {
+                if let Ok(data) =
+                    state.eval_parameter_arg(parameter, &self.project.stack_pointer_register)
+                {
                     possible_referenced_ids.append(&mut data.referenced_ids());
                 }
             }

cwe_checker_rs/src/analysis/pointer_inference/mod.rs

@@ -30,7 +30,7 @@ mod object_list;
 mod state;
 
 use context::Context;
-use state::State;
+pub use state::State;
 
 /// The version number of the analysis.
 const VERSION: &str = "0.1";

cwe_checker_rs/src/analysis/pointer_inference/state/access_handling.rs

@@ -191,6 +191,29 @@ impl State {
         }
     }
 
+    /// Evaluate the value of a parameter of an extern symbol for the given state.
+    pub fn eval_parameter_arg(
+        &self,
+        parameter: &Arg,
+        stack_pointer: &Variable,
+    ) -> Result<Data, Error> {
+        match parameter {
+            Arg::Register(var) => self.eval(&Expression::Var(var.clone())),
+            Arg::Stack { offset, size } => self.load_value(
+                &Expression::BinOp {
+                    op: BinOpType::IntAdd,
+                    lhs: Box::new(Expression::Var(stack_pointer.clone())),
+                    rhs: Box::new(Expression::Const(
+                        Bitvector::from_i64(*offset)
+                            .into_truncate(apint::BitWidth::from(stack_pointer.size))
+                            .unwrap(),
+                    )),
+                },
+                *size,
+            ),
+        }
+    }
+
     /// Check if an expression contains a use-after-free
     pub fn contains_access_of_dangling_memory(&self, def: &Def) -> bool {
         match def {

cwe_checker_rs/src/checkers.rs

+pub mod cwe_560;
 pub mod cwe_676;
 pub mod cwe_782;

cwe_checker_rs/src/checkers/cwe_560.rs

+//! This module implements a check for CWE-560: Use of umask() with chmod-style Argument.
+//!
+//! The program uses the system call umask(2) with arguments for chmod(2). For instance,
+//! instead of a reasonable value like 0022 a value like 0666 is passed. This may result in wrong
+//! read and/or write access to files and directories, which could be utilized to bypass
+//! protection mechanisms.
+//!
+//! See <https://cwe.mitre.org/data/definitions/560.html> for a detailed description.
+//!
+//! ## How the check works
+//!
+//! This check looks for umask calls and checks if they have a reasonable value, i.e. smaller than
+//! a certain value, currently set to 1000 and greater than a reasonable value for umask, currently set to 100.
+//!
+//! ## False Positives
+//!
+//! - A value deemed unreasonable by the check could theoretically be intended by the programmer.
+//! But these cases should be very rare in real programs, so be sure to double check them!
+//!
+//! ## False Negatives
+//!
+//! - If the input to umask is not defined in the basic block before the call, the check will not see it.
+//! However, a log message will be generated whenever the check is unable to determine the parameter value of umask.
+
+use crate::abstract_domain::{BitvectorDomain, DataDomain};
+use crate::analysis::pointer_inference::State;
+use crate::intermediate_representation::*;
+use crate::prelude::*;
+use crate::utils::log::{CweWarning, LogMessage};
+use crate::CweModule;
+
+pub static CWE_MODULE: CweModule = CweModule {
+    name: "CWE560",
+    version: "0.2",
+    run: check_cwe,
+};
+
+pub static UPPER_BOUND_CORRECT_UMASK_ARG_VALUE: u64 = 100;
+pub static UPPER_BOUND_CORRECT_CHMOD_ARG_VALUE: u64 = 1000;
+
+/// Compute the parameter value of umask out of the basic block right before the umask call.
+///
+/// The function uses the same `State` struct as the pointer inference analysis for the computation.
+fn get_umask_permission_arg(
+    block: &Term<Blk>,
+    umask_symbol: &ExternSymbol,
+    project: &Project,
+) -> Result<u64, Error> {
+    let stack_register = &project.stack_pointer_register;
+    let mut state = State::new(stack_register, block.tid.clone());
+
+    for def in block.term.defs.iter() {
+        match &def.term {
+            Def::Store { address, value } => {
+                let _ = state.handle_store(address, value);
+            }
+            Def::Assign { var, value } => {
+                let _ = state.handle_register_assign(var, value);
+            }
+            Def::Load { var, address } => {
+                let _ = state.handle_load(var, address);
+            }
+        }
+    }
+
+    let parameter = umask_symbol.get_unique_parameter()?;
+    let param_value = state.eval_parameter_arg(parameter, &project.stack_pointer_register)?;
+    if let DataDomain::Value(BitvectorDomain::Value(umask_arg)) = param_value {
+        Ok(umask_arg.try_to_u64()?)
+    } else {
+        Err(anyhow!("Parameter value unknown"))
+    }
+}
+
+/// Determine whether the given jump is a call to umask.
+fn is_call_to_umask(jmp: &Term<Jmp>, umask_tid: &Tid) -> bool {
+    matches!(&jmp.term, Jmp::Call { target, .. } if target == umask_tid)
+}
+
+/// Is the given argument value considered to be a chmod-style argument?
+fn is_chmod_style_arg(arg: u64) -> bool {
+    arg > UPPER_BOUND_CORRECT_UMASK_ARG_VALUE && arg <= UPPER_BOUND_CORRECT_CHMOD_ARG_VALUE
+}
+
+/// Generate the CWE warning for a detected instance of the CWE.
+fn generate_cwe_warning(sub: &Term<Sub>, jmp: &Term<Jmp>, permission_const: u64) -> CweWarning {
+    CweWarning::new(CWE_MODULE.name, CWE_MODULE.version,
+        format!("(Use of umask() with chmod-style Argument) Function {} calls umask with argument {:#o}", sub.term.name, permission_const))
+        .tids(vec![format!("{}", jmp.tid)])
+        .addresses(vec![jmp.tid.address.clone()])
+        .other(vec![vec![
+            "umask_arg".to_string(),
+            format!("{:#o}", permission_const),
+        ]])
+}
+
+/// Execute the CWE check.
+///
+/// For each call to umask we check whether the parameter value is a chmod-style parameter.
+/// If yes, generate a CWE warning.
+/// If the parameter value cannot be determined, generate a log message.
+///
+/// Only the basic block right before the umask call is evaluated when trying to determine the parameter value of umask.
+pub fn check_cwe(
+    project: &Project,
+    _cwe_params: &serde_json::Value,
+) -> (Vec<LogMessage>, Vec<CweWarning>) {
+    let mut cwes = Vec::new();
+    let mut log_messages = Vec::new();
+    if let Some(umask_symbol) = project
+        .program
+        .term
+        .extern_symbols
+        .iter()
+        .find(|symbol| symbol.name == "umask")
+    {
+        let umask_tid = &umask_symbol.tid;
+        for sub in project.program.term.subs.iter() {
+            for block in sub.term.blocks.iter() {
+                if let Some(jmp) = block
+                    .term
+                    .jmps
+                    .iter()
+                    .find(|jmp| is_call_to_umask(jmp, umask_tid))
+                {
+                    match get_umask_permission_arg(block, umask_symbol, project) {
+                        Ok(permission_const) => {
+                            if is_chmod_style_arg(permission_const) {
+                                cwes.push(generate_cwe_warning(sub, jmp, permission_const));
+                            }
+                        }
+                        Err(err) => {
+                            let log = LogMessage::new_info(format!(
+                                "Could not determine umask argument: {}",
+                                err
+                            ))
+                            .location(jmp.tid.clone())
+                            .source(CWE_MODULE.name);
+                            log_messages.push(log);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    (log_messages, cwes)
+}

cwe_checker_rs/src/lib.rs

@@ -51,6 +51,7 @@ impl std::fmt::Display for CweModule {
 /// Get a list of all known analysis modules.
 pub fn get_modules() -> Vec<&'static CweModule> {
     vec![
+        &crate::checkers::cwe_560::CWE_MODULE,
         &crate::checkers::cwe_782::CWE_MODULE,
         &crate::checkers::cwe_676::CWE_MODULE,
         &crate::analysis::pointer_inference::CWE_MODULE,

cwe_checker_rs/src/utils/log.rs

@@ -14,15 +14,19 @@ pub struct CweWarning {
 
 impl CweWarning {
     /// Creates a new CweWarning by only setting name, version and description
-    pub fn new(name: String, version: String, description: String) -> CweWarning {
+    pub fn new(
+        name: impl ToString,
+        version: impl ToString,
+        description: impl ToString,
+    ) -> CweWarning {
         CweWarning {
-            name,
-            version,
+            name: name.to_string(),
+            version: version.to_string(),
             addresses: Vec::new(),
             tids: Vec::new(),
             symbols: Vec::new(),
             other: Vec::new(),
-            description,
+            description: description.to_string(),
         }
     }
 

test/src/lib.rs

@@ -224,6 +224,32 @@ mod tests {
 
     #[test]
     #[ignore]
+    fn cwe_560() {
+        let mut error_log = Vec::new();
+        let mut tests = linux_test_cases("cwe_560", "CWE560");
+
+        mark_skipped(&mut tests, "arm", "gcc"); // The parameter is loaded from global memory (which is not supported yet)
+        mark_architecture_skipped(&mut tests, "mips64"); // TODO: Check reason for failure!
+        mark_architecture_skipped(&mut tests, "mips64el"); // TODO: Check reason for failure!
+        mark_skipped(&mut tests, "mips", "gcc"); // The parameter is loaded from global memory (which is not supported yet)
+        mark_skipped(&mut tests, "mipsel", "gcc"); // The parameter is loaded from global memory (which is not supported yet)
+        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("[CWE560]", 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_676() {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_676", "CWE676");