Unverified Commit d135a25e by Enkelmann Committed by GitHub

support more allocation and deallocation functions (#414)

parent 1b0f0b48
...@@ -89,6 +89,15 @@ ...@@ -89,6 +89,15 @@
] ]
] ]
}, },
"CWE416": {
"deallocation_symbols": [
"free",
"realloc",
"reallocarray",
"operator.delete",
"operator.delete[]"
]
},
"CWE426": { "CWE426": {
"_comment": "functions that change/drop privileges", "_comment": "functions that change/drop privileges",
"symbols": [ "symbols": [
...@@ -237,8 +246,11 @@ ...@@ -237,8 +246,11 @@
"malloc", "malloc",
"calloc", "calloc",
"realloc", "realloc",
"reallocarray",
"xmalloc", "xmalloc",
"strdup" "strdup",
"operator.new",
"operator.new[]"
] ]
}, },
"StringAbstraction": { "StringAbstraction": {
......
...@@ -34,7 +34,7 @@ pub struct Context<'a> { ...@@ -34,7 +34,7 @@ pub struct Context<'a> {
/// A map that maps abstract identifiers representing the values of parameters at callsites /// A map that maps abstract identifiers representing the values of parameters at callsites
/// to the corresponding value (in the context of the caller) according to the pointer inference analysis. /// to the corresponding value (in the context of the caller) according to the pointer inference analysis.
pub param_replacement_map: HashMap<AbstractIdentifier, Data>, pub param_replacement_map: HashMap<AbstractIdentifier, Data>,
/// A map that maps the TIDs of calls to allocatingfunctions (like malloc, realloc and calloc) /// A map that maps the TIDs of calls to allocating functions (like malloc, realloc and calloc)
/// to the value representing the size of the allocated memory object according to the pointer inference analysis. /// to the value representing the size of the allocated memory object according to the pointer inference analysis.
pub malloc_tid_to_object_size_map: HashMap<Tid, Data>, pub malloc_tid_to_object_size_map: HashMap<Tid, Data>,
/// A map that maps the TIDs of jump instructions to the function TID of the caller. /// A map that maps the TIDs of jump instructions to the function TID of the caller.
...@@ -238,14 +238,14 @@ fn compute_size_values_of_malloc_calls(analysis_results: &AnalysisResults) -> Ha ...@@ -238,14 +238,14 @@ fn compute_size_values_of_malloc_calls(analysis_results: &AnalysisResults) -> Ha
/// Compute the size value of a call to a malloc-like function according to the pointer inference and return it. /// Compute the size value of a call to a malloc-like function according to the pointer inference and return it.
/// Returns `None` if the called symbol is not an allocating function or the size computation for the symbol is not yet implemented. /// Returns `None` if the called symbol is not an allocating function or the size computation for the symbol is not yet implemented.
/// ///
/// Currently this function computes the size values for the symbols `malloc`, `realloc` and `calloc`. /// Currently this function computes the size values for the symbols `malloc`, `realloc`, `reallocarray`, `calloc` and the C++-`new` operator.
fn compute_size_value_of_malloc_like_call( fn compute_size_value_of_malloc_like_call(
jmp_tid: &Tid, jmp_tid: &Tid,
called_symbol: &ExternSymbol, called_symbol: &ExternSymbol,
pointer_inference: &PointerInference, pointer_inference: &PointerInference,
) -> Option<Data> { ) -> Option<Data> {
match called_symbol.name.as_str() { match called_symbol.name.as_str() {
"malloc" => { "malloc" | "operator.new" | "operator.new[]" => {
let size_param = &called_symbol.parameters[0]; let size_param = &called_symbol.parameters[0];
match pointer_inference.eval_parameter_arg_at_call(jmp_tid, size_param) { match pointer_inference.eval_parameter_arg_at_call(jmp_tid, size_param) {
Some(size_value) => Some(size_value), Some(size_value) => Some(size_value),
...@@ -259,6 +259,19 @@ fn compute_size_value_of_malloc_like_call( ...@@ -259,6 +259,19 @@ fn compute_size_value_of_malloc_like_call(
None => Some(Data::new_top(size_param.bytesize())), None => Some(Data::new_top(size_param.bytesize())),
} }
} }
"reallocarray" => {
let count_param = &called_symbol.parameters[1];
let size_param = &called_symbol.parameters[2];
match (
pointer_inference.eval_parameter_arg_at_call(jmp_tid, count_param),
pointer_inference.eval_parameter_arg_at_call(jmp_tid, size_param),
) {
(Some(count_value), Some(size_value)) => {
Some(count_value.bin_op(BinOpType::IntMult, &size_value))
}
_ => Some(Data::new_top(size_param.bytesize())),
}
}
"calloc" => { "calloc" => {
let count_param = &called_symbol.parameters[0]; let count_param = &called_symbol.parameters[0];
let size_param = &called_symbol.parameters[1]; let size_param = &called_symbol.parameters[1];
......
...@@ -12,12 +12,16 @@ use crate::prelude::*; ...@@ -12,12 +12,16 @@ use crate::prelude::*;
use crate::utils::log::CweWarning; use crate::utils::log::CweWarning;
use crate::utils::log::LogMessage; use crate::utils::log::LogMessage;
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::collections::BTreeSet;
/// The context struct for the fixpoint algorithm that contains references to the analysis results /// The context struct for the fixpoint algorithm that contains references to the analysis results
/// of other analyses used in this analysis. /// of other analyses used in this analysis.
pub struct Context<'a> { pub struct Context<'a> {
/// A pointer to the project struct. /// A pointer to the project struct.
pub project: &'a Project, pub project: &'a Project,
/// The names of extern functions that deallocate memory.
/// These functions will create dangling pointers during the analysis.
pub deallocation_symbols: BTreeSet<String>,
/// A pointer to the control flow graph. /// A pointer to the control flow graph.
pub graph: &'a Graph<'a>, pub graph: &'a Graph<'a>,
/// A pointer to the results of the pointer inference analysis. /// A pointer to the results of the pointer inference analysis.
...@@ -38,6 +42,7 @@ impl<'a> Context<'a> { ...@@ -38,6 +42,7 @@ impl<'a> Context<'a> {
analysis_results: &'b AnalysisResults<'a>, analysis_results: &'b AnalysisResults<'a>,
cwe_warning_collector: crossbeam_channel::Sender<WarningContext>, cwe_warning_collector: crossbeam_channel::Sender<WarningContext>,
log_collector: crossbeam_channel::Sender<LogMessage>, log_collector: crossbeam_channel::Sender<LogMessage>,
deallocation_symbols: BTreeSet<String>,
) -> Context<'a> ) -> Context<'a>
where where
'a: 'b, 'a: 'b,
...@@ -54,6 +59,7 @@ impl<'a> Context<'a> { ...@@ -54,6 +59,7 @@ impl<'a> Context<'a> {
}; };
Context { Context {
project: analysis_results.project, project: analysis_results.project,
deallocation_symbols,
graph: analysis_results.control_flow_graph, graph: analysis_results.control_flow_graph,
pointer_inference: analysis_results.pointer_inference.unwrap(), pointer_inference: analysis_results.pointer_inference.unwrap(),
function_signatures: analysis_results.function_signatures.unwrap(), function_signatures: analysis_results.function_signatures.unwrap(),
...@@ -312,7 +318,9 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont ...@@ -312,7 +318,9 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont
_ => None, _ => None,
} { } {
match extern_symbol.name.as_str() { match extern_symbol.name.as_str() {
"free" => self.handle_call_to_free(&mut state, &call.tid, extern_symbol), dealloc_sym if self.deallocation_symbols.contains(dealloc_sym) => {
self.handle_call_to_free(&mut state, &call.tid, extern_symbol)
}
extern_symbol_name => { extern_symbol_name => {
if let Some(warning_causes) = self.collect_cwe_warnings_of_call_params( if let Some(warning_causes) = self.collect_cwe_warnings_of_call_params(
&mut state, &mut state,
......
...@@ -17,12 +17,23 @@ ...@@ -17,12 +17,23 @@
//! To prevent duplicate CWE warnings with the same root cause //! To prevent duplicate CWE warnings with the same root cause
//! the check also keeps track of objects for which a CWE warning was already generated. //! the check also keeps track of objects for which a CWE warning was already generated.
//! //!
//! ### Symbols configurable in config.json
//!
//! The `deallocation_symbols` are the names of extern functions that deallocate memory.
//! The check always assumes that the first parameter of such a function is the memory object to be freed.
//! The check also assumes that memory is always freed by such a call,
//! which can lead to false positive warnings for functions like `realloc`, where the memory object may not be freed by the call.
//!
//! ## False Positives //! ## False Positives
//! //!
//! - Since the analysis is not path-sensitive, infeasible paths may lead to false positives. //! - Since the analysis is not path-sensitive, infeasible paths may lead to false positives.
//! - Any analysis imprecision of the pointer inference analysis //! - Any analysis imprecision of the pointer inference analysis
//! that leads to assuming that a pointer can target more memory objects that it actually can target //! that leads to assuming that a pointer can target more memory objects that it actually can target
//! may lead to false positive CWE warnings in this check. //! may lead to false positive CWE warnings in this check.
//! - For extern functions that may or may not release memory,
//! the check will produce false positives if the original pointer is used after calling the function.
//! For example, `realloc` may return NULL, in which case it will not free memory and the original pointer remains valid.
//! But the check will flag any access to the original pointer as a potential CWE, regardless of the return value of `realloc`.
//! //!
//! ## False Negatives //! ## False Negatives
//! //!
...@@ -30,8 +41,7 @@ ...@@ -30,8 +41,7 @@
//! Subsequently, CWEs corresponding to arrays of memory objects are not detected. //! Subsequently, CWEs corresponding to arrays of memory objects are not detected.
//! - Memory objects not tracked by the Pointer Inference analysis or pointer targets missed by the Pointer Inference //! - Memory objects not tracked by the Pointer Inference analysis or pointer targets missed by the Pointer Inference
//! may lead to missed CWEs in this check. //! may lead to missed CWEs in this check.
//! - The analysis currently only tracks pointers to objects that were freed by a call to `free`. //! - Pointers freed by other operations than calls to the deallocation symbols contained in the config.json will be missed by the analysis.
//! If a memory object is freed by another external function then this may lead to false negatives in this check.
use std::collections::BTreeSet; use std::collections::BTreeSet;
use std::collections::HashMap; use std::collections::HashMap;
...@@ -55,6 +65,14 @@ pub static CWE_MODULE: CweModule = CweModule { ...@@ -55,6 +65,14 @@ pub static CWE_MODULE: CweModule = CweModule {
run: check_cwe, run: check_cwe,
}; };
/// The configuration struct
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
pub struct Config {
/// The names of symbols that free memory (e.g. the "free" function of C).
/// The analysis always assumes that the memory object to be freed is the first parameter of the function.
deallocation_symbols: Vec<String>,
}
mod context; mod context;
use context::Context; use context::Context;
mod state; mod state;
...@@ -68,11 +86,18 @@ use state::State; ...@@ -68,11 +86,18 @@ use state::State;
/// Returns collected log messages and CWE warnings. /// Returns collected log messages and CWE warnings.
pub fn check_cwe( pub fn check_cwe(
analysis_results: &AnalysisResults, analysis_results: &AnalysisResults,
_config: &serde_json::Value, config_json: &serde_json::Value,
) -> (Vec<LogMessage>, Vec<CweWarning>) { ) -> (Vec<LogMessage>, Vec<CweWarning>) {
let config: Config = serde_json::from_value(config_json.clone()).unwrap();
let deallocation_symbols = config.deallocation_symbols.iter().cloned().collect();
let (cwe_warning_sender, cwe_warning_receiver) = crossbeam_channel::unbounded(); let (cwe_warning_sender, cwe_warning_receiver) = crossbeam_channel::unbounded();
let (log_sender, log_receiver) = crossbeam_channel::unbounded(); let (log_sender, log_receiver) = crossbeam_channel::unbounded();
let context = Context::new(analysis_results, cwe_warning_sender, log_sender); let context = Context::new(
analysis_results,
cwe_warning_sender,
log_sender,
deallocation_symbols,
);
let mut fixpoint_computation = let mut fixpoint_computation =
crate::analysis::forward_interprocedural_fixpoint::create_computation(context, None); crate::analysis::forward_interprocedural_fixpoint::create_computation(context, None);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment