Skip to content

Commit

Permalink
Denote limited interprocedurality for CWE-119 check in doc comment
Browse files Browse the repository at this point in the history
  • Loading branch information
Enkelmann committed Dec 12, 2023
1 parent 490a95f commit 698f0bb
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ impl<'a> Context<'a> {
object_id: &AbstractIdentifier,
current_stack_frame_id: &AbstractIdentifier,
) -> (Option<BoundsMetadata>, Option<BoundsMetadata>) {
// FIXME: The malloc-tid-to-object-size-map check does not work anymore,
// because we do not use path hints in the PointerInference anymore.
if self
.malloc_tid_to_object_size_map
.contains_key(object_id.get_tid())
Expand Down
2 changes: 2 additions & 0 deletions src/cwe_checker_lib/src/checkers/cwe_119/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ impl<'a> Context<'a> {
/// then the absolute value is used and unknown origins of the size value are ignored.
/// If more than one possible absolute value for the size is found then the minimum value for the size is returned.
pub fn compute_size_of_heap_object(&self, object_id: &AbstractIdentifier) -> BitvectorDomain {
// FIXME: We use path hints, which are not longer provided by the PointerInference, to substitute some values.
// We either have to change that or make sure that we provide the path hints ourselves.
if let Some(object_size) = self.malloc_tid_to_object_size_map.get(object_id.get_tid()) {
let fn_tid_at_malloc_call = self.call_to_caller_fn_map[object_id.get_tid()].clone();
let object_size = self.recursively_substitute_param_values_context_sensitive(
Expand Down
21 changes: 19 additions & 2 deletions src/cwe_checker_lib/src/checkers/cwe_119/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@
//!
//! The check uses the results of the [Pointer Inference analysis](`crate::analysis::pointer_inference`)
//! to check whether any memory accesses may point outside of the bounds of the corresponding memory objects.
//! For this the results of the Pointer Inference analysis are aggregated interprocedurally.
//! Additionally, the check uses a lightweight intraprocedural dataflow fixpoint computation
//! Additionally, the check uses a lightweight dataflow fixpoint computation
//! to ensure that for each memory object only the first access outside of its bounds is flagged as a CWE.
//!
//! Currently, the check is only partially interprocedural.
//! Bounds of parameter objects can be detected, but bounds of memory objects created in called functions
//! (other than the standard allocation functions) will not be detected.
//!
//! ## False Positives
//!
//! - Any analysis imprecision of the Pointer Inference analysis may lead to false positive results in this check.
Expand All @@ -40,6 +43,20 @@
//! this still may miss buffer overflows occuring in the called function.
//! - Right now the check only considers buffers on the stack or the heap, but not buffers in global memory.
//! Thus corresponding overflows of buffers in global memory are not detected.
//! - Since the check is only partially interprocedural at the moment,
//! it will miss object sizes of objects created in called functions.
//! For example, if allocations are wrapped in simple wrapper functions,
//! the analysis will miss overflows for corresponding objects, because it cannot determine their object sizes.
// FIXME: The current implementation uses path hints for memory object IDs to determine object sizes interprocedurally.
// But the number of path hint combinations can grow exponentially
// with the call depth to the actual allocation size of a callee-created object.
// This led to state explosion in the PointerInference and thus path hints are not longer provided by the PointerInference.
// But without the path hints that this analysis depended on, the check can only resolve sizes of parameter objects,
// but not of objects returned from called functions (other than the standard allocation functions).
// A future implementation needs a better way to determine object sizes interprocedurally,
// probably depending on several fixpoint computations to circumvent the state explosion problems
// that the old implementation is vulnerable to.

use crate::analysis::pointer_inference::Data;
use crate::prelude::*;
Expand Down

0 comments on commit 698f0bb

Please sign in to comment.