Skip to content
60 changes: 47 additions & 13 deletions resolve-cveassert/libresolve/src/remediate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use libc::{
c_char, c_void, calloc, free, malloc, realloc, strdup, strlen, strndup, strnlen,
};

use crate::shadowobjs::{ALIVE_OBJ_LIST, AllocType, FREED_OBJ_LIST, Vaddr};
use crate::shadowobjs::{SHADOW_STACK, ALIVE_OBJ_LIST, AllocType, FREED_OBJ_LIST, Vaddr};

use log::{info, warn};

Expand All @@ -16,24 +16,25 @@ use log::{info, warn};
#[unsafe(no_mangle)]
pub extern "C" fn __resolve_alloca(ptr: *mut c_void, size: usize) -> () {
let base = ptr as Vaddr;
{
let mut obj_list = ALIVE_OBJ_LIST.lock();
obj_list.add_shadow_object(AllocType::Stack, base, size);
}

SHADOW_STACK.with_borrow_mut(
|ss|
ss.add_shadow_object(base, size)
);

info!("[STACK] Object allocated with size: {size}, address: 0x{base:x}");
}

#[unsafe(no_mangle)]
pub extern "C" fn __resolve_invalidate_stack(base: *mut c_void) {
pub extern "C" fn __resolve_invalidate_stack_range(base: *mut c_void, size: usize) {
let base = base as Vaddr;

{
let mut obj_list = ALIVE_OBJ_LIST.lock();
obj_list.invalidate_at(base);
}
SHADOW_STACK.with_borrow_mut(
|ss|
ss.invalidate_at(base, size)
);

info!("[STACK] Free addr 0x{base:x}");
info!("[STACK] Free addr 0x{base:x} size {size}");
}

/**
Expand Down Expand Up @@ -251,6 +252,18 @@ pub struct ShadowObjBounds {
pub limit: *mut c_void,
}

impl ShadowObjBounds {
pub fn null() -> Self {
ShadowObjBounds { base: std::ptr::null_mut(), limit: std::ptr::null_mut() }
}
}

impl From<&crate::shadowobjs::ShadowObject> for ShadowObjBounds {
fn from(sobj: &crate::shadowobjs::ShadowObject) -> Self {
ShadowObjBounds { base: sobj.base as *mut c_void, limit: sobj.limit as *mut c_void }
}
}

/**
* @brief - Helper function that queries shadow obj list
* to find a shadow obj where the ptr fits within
Expand All @@ -261,13 +274,34 @@ pub struct ShadowObjBounds {
* shadow object as pointers
*/
#[unsafe(no_mangle)]
pub extern "C" fn __resolve_get_bounds_stack(ptr: *mut c_void) -> ShadowObjBounds {
return SHADOW_STACK.with_borrow(
|ss| {
return match ss.search_intersection(ptr as Vaddr) {
Some(sobj) => { sobj.into() }
None => { ShadowObjBounds::null() }
}
}
);
}

/**
* @brief - Helper function that queries stack shadow obj list
* to find a shadow obj where the ptr fits within
* its bounds of allocation
* @input
* - ptr: ptr to allocation
* @return struct containing the base and limit of the
* shadow object as pointers
*/
#[unsafe(no_mangle)]
pub extern "C" fn __resolve_get_bounds(ptr: *mut c_void) -> ShadowObjBounds {
let sobj_table = ALIVE_OBJ_LIST.lock();
let Some(sobj) = sobj_table.search_intersection(ptr as Vaddr) else {
return ShadowObjBounds { base: std::ptr::null_mut(), limit: std::ptr::null_mut() }
return ShadowObjBounds::null();
};

return ShadowObjBounds { base: sobj.base as *mut c_void, limit: sobj.limit as *mut c_void }
return sobj.into();
Comment thread
zoogies marked this conversation as resolved.
}

#[unsafe(no_mangle)]
Expand Down
181 changes: 167 additions & 14 deletions resolve-cveassert/libresolve/src/shadowobjs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// LGPL-3; See LICENSE.txt in the repo root for details.

use crate::MutexWrap;
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::ops::RangeInclusive;
use std::ops::Bound::Included;
Expand Down Expand Up @@ -32,6 +33,15 @@ pub struct ShadowObject {
}

impl ShadowObject {
pub fn new(ty: AllocType, base: Vaddr, size: usize) -> Self {
Self {
alloc_type: ty,
base,
limit: ShadowObject::limit(base, size),
size
}
}

/// Returns the base + limit of this shadow object as RangeInclusive
///
/// Useful for querying contains
Expand All @@ -44,23 +54,19 @@ impl ShadowObject {
self.bounds().contains(&addr)
}

// pub fn contains_region(&self, base: Vaddr, limit: Vaddr) -> bool {
// self.contains(base) && self.contains(limit)
// }

/// Computes the size of the shadow object from its base and limit
pub fn size(&self) -> usize {
self.size
}

/// Compute a limit from base and size
pub fn limit(base: Vaddr, size: usize) -> Vaddr {
if size == 0 { base } else { base + size - 1 }
if size == 0 { base } else { base.saturating_add(size - 1) }
}

/// Compute the sentinel pointer value for this object, 1 past its limit
pub fn past_limit(&self) -> Vaddr {
self.limit + 1
self.limit.saturating_add(1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearly the existing behavior was not the right one, but what is the argument for saturating add? Can we record it in a comment?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was mostly just to be consistent with the other places I used saturating add for Vaddr offset computation. Happy to add a comment to each instance if you have a suggestion for what to say. Maybe "avoid overflow". Realistically though this should never actually saturate

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just panic on overflow? We should probably revisit the overflow behavior everywhere...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could remove the saturating calls and just force it to panic with overflow-checks = true in Cargo.toml? Unsure what the correct behavior is here that doesn't cost performance.

}
}

Expand All @@ -77,13 +83,7 @@ impl ShadowObjectTable {

/// Adds a new shadow object to the object list, replacing any existing object at `base`
pub fn add_shadow_object(&mut self, alloc_type: AllocType, base: Vaddr, size: usize) {
let sobj = ShadowObject {
alloc_type,
base,
limit: ShadowObject::limit(base, size),
size,
};
self.table.insert(base, sobj);
self.table.insert(base, ShadowObject::new(alloc_type, base, size));
}

/// Removes the shadow object with base address equal to `base`.
Expand Down Expand Up @@ -117,6 +117,159 @@ impl ShadowObjectTable {
pub static ALIVE_OBJ_LIST: MutexWrap<ShadowObjectTable> = MutexWrap::new(ShadowObjectTable::new());
pub static FREED_OBJ_LIST: MutexWrap<ShadowObjectTable> = MutexWrap::new(ShadowObjectTable::new());

// data must be ordered descending (downward growing stack on x86)
// so push/pop are O(1) at the end.
#[derive(Default)]
pub struct ShadowStack {
data: Vec<ShadowObject>
}
Comment thread
zoogies marked this conversation as resolved.

enum LookupError {
ObjectNotFound,
}

impl ShadowStack {
pub fn new() -> Self {
Self::default()
}

pub fn add_shadow_object(&mut self, base: Vaddr, size: usize) {
let new_end = base.checked_add(size)
.expect("add_shadow_object: object overflows the address space"); // exclusive

let Ok((reused, idx)) = self.get_at(base) else {
// most common: pushing a new obj onto the end of the stack
assert!(self.data.last().map_or(true, |top| new_end <= top.base),
"ShadowStack::add_shadow_object: new object overlaps the stack top");
self.data.push(ShadowObject::new(AllocType::Stack, base, size));
return;
};

let slot_base = reused.base;
let slot_end = slot_base + reused.size();
let reused_live = reused.alloc_type != AllocType::Unallocated;

// also common: new object is being pushed after program has fallen
// back a few stack frames. Overwrite and truncate.
if reused_live {
assert!(slot_base == base,
"ShadowStack::add_shadow_object: re-push lands inside a live object");
assert!(idx == 0 || new_end <= self.data[idx - 1].base,
"ShadowStack::add_shadow_object: object overlaps the frame above");
self.data[idx] = ShadowObject::new(AllocType::Stack, base, size);
self.data.truncate(idx + 1); // drop everything more recent
}
else {
// least common: stack re-use (new alloca inside previously invalidated region)
assert!(new_end <= slot_end,
"ShadowStack::add_shadow_object: object overflows its slot");
self.data[idx] = ShadowObject::new(AllocType::Stack, base, size);

// retain invalidated padding around object
if slot_base < base {
self.data.insert(idx + 1, ShadowObject::new(AllocType::Unallocated, slot_base, base - slot_base));
}
if new_end < slot_end {
self.data.insert(idx, ShadowObject::new(AllocType::Unallocated, new_end, slot_end - new_end));
}
}
}

fn get_at(&self, addr: Vaddr) -> Result<(&ShadowObject, usize), LookupError> {
use LookupError::*;

let n = self.data.len();
if n == 0 { return Err(ObjectNotFound); }

// fast path: the top frame (should be) the most common lookup target.
let top_idx = n - 1;
let top = &self.data[top_idx];
if top.contains(addr) {
return Ok((top, top_idx));
}

// easy out: below every tracked object
if addr < top.base {
return Err(ObjectNotFound); // should we return a seperate ObjectOutOfBounds?
}

// binary search the shadow stack for value
let idx = self.data.partition_point(|o| o.base > addr);
let obj = &self.data[idx];
if obj.contains(addr) {
Ok((obj, idx))
} else {
Err(ObjectNotFound)
}
}

/*
Invalidate the address range [base, base + length).

`base` and `base + length` must each land exactly on a tracked object
boundary. The range may span several whole objects (live or dead), but
it may not bisect one. The spanned entries are dropped and replaced by
a single dead marker.
*/
pub fn invalidate_at(&mut self, base: Vaddr, length: usize) {
if length == 0 { return; }
let end = base.checked_add(length)
.expect("invalidate_at: range overflows the address space"); // exclusive

// entry holding `base` (lowest address in the range)
let (start_idx, lo_base) = match self.get_at(base) {
Ok((v, idx)) => (idx, v.base),
Err(_) => { debug_assert!(false, "invalidate_at: untracked base"); return; }
};
assert!(lo_base == base,
"invalidate_at: range start 0x{base:x} is not an object boundary");

// entry holding `end - 1` (highest address in the range)
let (end_idx, hi_end) = match self.get_at(end - 1) {
Ok((v, idx)) => (idx, v.base + v.size()),
Err(_) => { debug_assert!(false, "invalidate_at: untracked limit"); return; }
};
assert!(hi_end == end,
"invalidate_at: range end 0x{end:x} is not an object boundary");

debug_assert!(end_idx <= start_idx);

let was_top = start_idx == self.data.len() - 1;
self.data.drain(end_idx..=start_idx);

// if we didn't reach the top, leave a dead marker for the invalidated range
if !was_top {
self.data.insert(end_idx, ShadowObject::new(AllocType::Unallocated, base, length));
}
}

// TODO: Does it make more sense to return something explicit
// when we search and get an invalidated stack object?
pub fn search_intersection(&self, addr: Vaddr) -> Option<&ShadowObject> {
// exact containment in a live object
if let Ok((sobj, _)) = self.get_at(addr)
&& sobj.alloc_type != AllocType::Unallocated
{
return Some(sobj);
}

// edge case: GEP remediation one-past
if let Some(prev) = addr.checked_sub(1)
&& let Ok((sobj, _)) = self.get_at(prev)
&& sobj.alloc_type != AllocType::Unallocated
&& sobj.past_limit() == addr
{
return Some(sobj);
}

None
}
}

thread_local! {
pub static SHADOW_STACK: RefCell<ShadowStack> = RefCell::new(ShadowStack::new());
}

#[cfg(test)]
mod tests {
use crate::shadowobjs::{AllocType, ShadowObjectTable};
Expand Down Expand Up @@ -184,4 +337,4 @@ mod tests {
// let table = ShadowObjectTable::new();
//table.bounds(0xDEADBEEF).unwrap(); // should panic since there is no interesection
}
}
}
Loading