T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Validate faulting addresses are valid to fault on (#6028)

Browse Source 
* Validate faulting addresses are valid to fault on

This commit adds a defense-in-depth measure to Wasmtime which is
intended to mitigate the impact of CVEs such as GHSA-ff4p-7xrq-q5r8.
Currently Wasmtime will catch `SIGSEGV` signals for WebAssembly code so
long as the instruction which faulted is an allow-listed instruction
(aka has a trap code listed for it). With the recent security issue,
however, the problem was that a wasm guest could exploit a compiler bug
to access memory outside of its sandbox. If the access was successful
there's no real way to detect that, but if the access was unsuccessful
then Wasmtime would happily swallow the `SIGSEGV` and report a nominal
trap. To embedders, this might look like nothing is going awry.

The new strategy implemented here in this commit is to attempt to be
more robust towards these sorts of failures. When a `SIGSEGV` is raised
the faulting pc is recorded but additionally the address of the
inaccessible location is also record. After the WebAssembly stack is
unwound and control returns to Wasmtime which has access to a `Store`
Wasmtime will now use this inaccessible faulting address to translate it
to a wasm address. This process should be guaranteed to succeed as
WebAssembly should only be able to access a well-defined region of
memory for all linear memories in a `Store`.

If no linear memory in a `Store` could contain the faulting address,
then Wasmtime now prints a scary message and aborts the process. The
purpose of this is to catch these sorts of bugs, make them very loud
errors, and hopefully mitigate impact. This would continue to not
mitigate the impact of a guest successfully loading data outside of its
sandbox, but if a guest was doing a sort of probing strategy trying to
find valid addresses then any invalid access would turn into a process
crash which would immediately be noticed by embedders.

While I was here I went ahead and additionally took a stab at #3120.
Traps due to `SIGSEGV` will now report the size of linear memory and the
address that was being accessed in addition to the bland "access out of
bounds" error. While this is still somewhat bland in the context of a
high level source language it's hopefully at least a little bit more
actionable for some. I'll note though that this isn't a guaranteed
contextual message since only the default configuration for Wasmtime
generates `SIGSEGV` on out-of-bounds memory accesses. Dynamically
bounds-checked configurations, for example, don't do this.

Testing-wise I unfortunately am not aware of a great way to test this.
The closet equivalent would be something like an `unsafe` method
`Config::allow_wasm_sandbox_escape`. In lieu of adding tests, though, I
can confirm that during development the crashing messages works just
fine as it took awhile on macOS to figure out where the faulting address
was recorded in the exception information which meant I had lots of
instances of recording an address of a trap not accessible from wasm.

* Fix tests

* Review comments

* Fix compile after refactor

* Fix compile on macOS

* Fix trap test for s390x

s390x rounds faulting addresses to 4k boundaries.
This commit is contained in:
Alex Crichton
2023-03-17 09:52:54 -05:00
committed by GitHub
parent a66b3e1ab6
commit 28371bfd40
15 changed files with 325 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
crates/runtime/src/traphandlers/macos.rs
View File

@@ -75,7 +75,18 @@ mod mach_addons {
pub static NDR_record: NDR_record_t;
}
#[repr(C)]
// Note that this is copied from Gecko at
//
// https://searchfox.org/mozilla-central/rev/ed93119be4818da1509bbcb7b28e245853eeedd5/js/src/wasm/WasmSignalHandlers.cpp#583-601
//
// which distinctly diverges from the actual version of this in the header
// files provided by macOS, notably in the `code` field which uses `i64`
// instead of `i32`.
//
// Also note the `packed(4)` here which forcibly decreases alignment to 4 to
// additionally match what mach expects (apparently, I wish I had a better
// reference for this).
#[repr(C, packed(4))]
#[allow(dead_code)]
#[derive(Copy, Clone, Debug)]
pub struct __Request__exception_raise_t {
@@ -88,6 +99,11 @@ mod mach_addons {
pub NDR: NDR_record_t,
pub exception: exception_type_t,
pub codeCnt: mach_msg_type_number_t,
// Note that this is a divergence from the C headers which use
// `integer_t` here for this field which is a `c_int`. That isn't
// actually reflecting reality apparently though because if `c_int` is
// used here then the structure is too small to receive a message.
pub code: [i64; 2],
}
@@ -172,6 +188,7 @@ pub unsafe fn platform_init() {
// This is largely just copied from SpiderMonkey.
#[repr(C)]
#[allow(dead_code)]
#[derive(Debug)]
struct ExceptionRequest {
body: __Request__exception_raise_t,
trailer: mach_msg_trailer_t,
@@ -248,6 +265,16 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
_ => return false,
}
// For `EXC_BAD_ACCESS` the faulting address is listed as the "subcode" in
// the second `code` field. If we're ever interested in it the first code
// field has a `kern_return_t` describing the kind of failure (e.g. SIGSEGV
// vs SIGBUS), but we're not interested in that right now.
let (fault1, fault2) = if request.body.exception as u32 == EXC_BAD_ACCESS {
(1, request.body.code[1] as usize)
} else {
(0, 0)
};
// Depending on the current architecture various bits and pieces of this
// will change. This is expected to get filled out for other macos
// platforms as necessary.
@@ -279,7 +306,7 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
state.__rbp as usize,
);
let resume = |state: &mut ThreadState, pc: usize, fp: usize| {
let resume = |state: &mut ThreadState, pc: usize, fp: usize, fault1: usize, fault2: usize| {
// The x86_64 ABI requires a 16-byte stack alignment for
// functions, so typically we'll be 16-byte aligned. In this
// case we simulate a `call` instruction by decrementing the
@@ -306,6 +333,8 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
state.__rip = unwind as u64;
state.__rdi = pc as u64;
state.__rsi = fp as u64;
state.__rdx = fault1 as u64;
state.__rcx = fault2 as u64;
};
let mut thread_state = ThreadState::new();
} else if #[cfg(target_arch = "aarch64")] {
@@ -318,7 +347,7 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
state.__fp as usize,
);
let resume = |state: &mut ThreadState, pc: usize, fp: usize| {
let resume = |state: &mut ThreadState, pc: usize, fp: usize, fault1: usize, fault2: usize| {
// Clobber LR with the faulting PC, so unwinding resumes at the
// faulting instruction. The previous value of LR has been saved
// by the callee (in Cranelift generated code), so no need to
@@ -329,6 +358,8 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
// it looks like a call to unwind.
state.__x[0] = pc as u64;
state.__x[1] = fp as u64;
state.__x[2] = fault1 as u64;
state.__x[3] = fault2 as u64;
state.__pc = unwind as u64;
};
let mut thread_state = mem::zeroed::<ThreadState>();
@@ -373,7 +404,7 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
// force the thread itself to trap. The thread's register state is
// configured to resume in the `unwind` function below, we update the
// thread's register state, and then we're off to the races.
resume(&mut thread_state, pc as usize, fp);
resume(&mut thread_state, pc as usize, fp, fault1, fault2);
let kret = thread_set_state(
origin_thread,
thread_state_flavor,
@@ -390,10 +421,20 @@ unsafe fn handle_exception(request: &mut ExceptionRequest) -> bool {
/// a native backtrace once we've switched back to the thread itself. After
/// the backtrace is captured we can do the usual `longjmp` back to the source
/// of the wasm code.
unsafe extern "C" fn unwind(wasm_pc: *const u8, wasm_fp: usize) -> ! {
unsafe extern "C" fn unwind(
wasm_pc: *const u8,
wasm_fp: usize,
has_faulting_addr: usize,
faulting_addr: usize,
) -> ! {
let jmp_buf = tls::with(|state| {
let state = state.unwrap();
state.set_jit_trap(wasm_pc, wasm_fp);
let faulting_addr = if has_faulting_addr != 0 {
Some(faulting_addr)
} else {
None
};
state.set_jit_trap(wasm_pc, wasm_fp, faulting_addr);
state.jmp_buf.get()
});
debug_assert!(!jmp_buf.is_null());

6
crates/runtime/src/traphandlers/unix.rs
View File

@@ -101,7 +101,11 @@ unsafe extern "C" fn trap_handler(
if jmp_buf as usize == 1 {
return true;
}
info.set_jit_trap(pc, fp);
let faulting_addr = match signum {
libc::SIGSEGV | libc::SIGBUS => Some((*siginfo).si_addr() as usize),
_ => None,
};
info.set_jit_trap(pc, fp, faulting_addr);
// On macOS this is a bit special, unfortunately. If we were to
// `siglongjmp` out of the signal handler that notably does
// *not* reset the sigaltstack state of our signal handler. This

12
crates/runtime/src/traphandlers/windows.rs
View File

@@ -62,13 +62,23 @@ unsafe extern "system" fn exception_handler(exception_info: *mut EXCEPTION_POINT
compile_error!("unsupported platform");
}
}
// For access violations the first element in `ExceptionInformation` is
// an indicator as to whether the fault was a read/write. The second
// element is the address of the inaccessible data causing this
// violation.
let faulting_addr = if record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION {
assert!(record.NumberParameters >= 2);
Some(record.ExceptionInformation[1])
} else {
None
};
let jmp_buf = info.take_jmp_buf_if_trap(ip, |handler| handler(exception_info));
if jmp_buf.is_null() {
ExceptionContinueSearch
} else if jmp_buf as usize == 1 {
ExceptionContinueExecution
} else {
info.set_jit_trap(ip, fp);
info.set_jit_trap(ip, fp, faulting_addr);
wasmtime_longjmp(jmp_buf)
}
})