Unverified Commit 3cc6cf5b by Enkelmann Committed by GitHub

fix stack frame bounds computation in CWE-119 check (#326)

parent 19e7a9ef
...@@ -164,6 +164,8 @@ impl<'a> Context<'a> { ...@@ -164,6 +164,8 @@ impl<'a> Context<'a> {
if !already_handled_ids.insert(id.clone()) if !already_handled_ids.insert(id.clone())
|| !id.get_path_hints().is_empty() || !id.get_path_hints().is_empty()
|| !subs_list.contains_key(id.get_tid()) || !subs_list.contains_key(id.get_tid())
|| *id.get_location()
== AbstractLocation::Register(self.project.stack_pointer_register.clone())
{ {
// ID was already present in `already_handled_ids` or it is not a parameter ID // ID was already present in `already_handled_ids` or it is not a parameter ID
replacement_map.insert( replacement_map.insert(
......
...@@ -182,8 +182,8 @@ impl State { ...@@ -182,8 +182,8 @@ impl State {
.unwrap() .unwrap()
.get_stack_params_total_size(); .get_stack_params_total_size();
upper_bound = upper_bound upper_bound = upper_bound
.map(|old_bound| std::cmp::min(old_bound, stack_frame_upper_bound)) .map(|old_bound| std::cmp::min(old_bound, stack_frame_upper_bound - offset))
.or(Some(stack_frame_upper_bound)); .or(Some(stack_frame_upper_bound - offset));
// We do not set a lower bound since we do not know the concrete call site for stack pointers, // We do not set a lower bound since we do not know the concrete call site for stack pointers,
// which we would need to determine a correct lower bound. // which we would need to determine a correct lower bound.
} }
...@@ -345,12 +345,19 @@ pub mod tests { ...@@ -345,12 +345,19 @@ pub mod tests {
fn test_compute_bounds_of_param_id() { fn test_compute_bounds_of_param_id() {
let mut context = Context::mock_x64(); let mut context = Context::mock_x64();
let param_id = AbstractIdentifier::mock("func", "RDI", 8); let param_id = AbstractIdentifier::mock("func", "RDI", 8);
let param_id_2 = AbstractIdentifier::mock("func", "RSI", 8);
let callsite_id = AbstractIdentifier::mock("callsite_id", "RDI", 8); let callsite_id = AbstractIdentifier::mock("callsite_id", "RDI", 8);
let callsite_id_2 = AbstractIdentifier::mock("callsite_id", "RSI", 8);
let malloc_call_id = AbstractIdentifier::mock("malloc_call", "RAX", 8); let malloc_call_id = AbstractIdentifier::mock("malloc_call", "RAX", 8);
let main_stack_id = AbstractIdentifier::mock("main", "RSP", 8);
let param_value = Data::from_target(malloc_call_id.clone(), Bitvector::from_i64(2).into()); let param_value = Data::from_target(malloc_call_id.clone(), Bitvector::from_i64(2).into());
let param_replacement_map = HashMap::from([(callsite_id, param_value.clone())]); let param_value_2 =
Data::from_target(main_stack_id.clone(), Bitvector::from_i64(-10).into());
let param_replacement_map = HashMap::from([
(callsite_id, param_value.clone()),
(callsite_id_2, param_value_2.clone()),
]);
let callee_to_callsites_map = let callee_to_callsites_map =
HashMap::from([(Tid::new("func"), HashSet::from([Tid::new("callsite_id")]))]); HashMap::from([(Tid::new("func"), HashSet::from([Tid::new("callsite_id")]))]);
context.param_replacement_map = param_replacement_map; context.param_replacement_map = param_replacement_map;
...@@ -367,7 +374,7 @@ pub mod tests { ...@@ -367,7 +374,7 @@ pub mod tests {
&FunctionSignature::mock_x64(), &FunctionSignature::mock_x64(),
context.project, context.project,
); );
// Test bound computation if the param gets resolved to a heap object
state.compute_bounds_of_param_id(&param_id, &context); state.compute_bounds_of_param_id(&param_id, &context);
assert_eq!(state.object_lower_bounds.len(), 2); assert_eq!(state.object_lower_bounds.len(), 2);
assert_eq!( assert_eq!(
...@@ -378,5 +385,15 @@ pub mod tests { ...@@ -378,5 +385,15 @@ pub mod tests {
state.object_upper_bounds[&AbstractIdentifier::mock("func", "RDI", 8)], state.object_upper_bounds[&AbstractIdentifier::mock("func", "RDI", 8)],
Bitvector::from_i64(40).into() Bitvector::from_i64(40).into()
); );
// Test bound computation if the param gets resolved to a caller stack frame
state.compute_bounds_of_param_id(&param_id_2, &context);
assert_eq!(
state.object_lower_bounds[&AbstractIdentifier::mock("func", "RSI", 8)],
BitvectorDomain::new_top(ByteSize::new(8))
);
assert_eq!(
state.object_upper_bounds[&AbstractIdentifier::mock("func", "RSI", 8)],
Bitvector::from_i64(10).into()
);
} }
} }
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