security: harden wasm ffi
ober
aec9541fead7b04c87c0b2537695ab7273cb5fa6
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -40,10 +40,10 @@ Latest summary from `tools/ffi-audit-report.ss`: (rust-file-count 29) (c-file-count 5) (no-mangle-exports 196) - (unsafe-sites 429) - (unsafe-sites-with-nearby-safety-comment 197) - (unsafe-sites-without-nearby-safety-comment 232) - (unsafe-comment-review-sites 232) + (unsafe-sites 427) + (unsafe-sites-with-nearby-safety-comment 198) + (unsafe-sites-without-nearby-safety-comment 229) + (unsafe-comment-review-sites 229) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -123,9 +123,13 @@ buffers, documents result-slot and output-buffer writes, and permits zero-sized output buffers without manufacturing null slices. `landlock.rs` now validates path and TCP port inputs before issuing syscalls, rejects interior-NUL path truncation, documents Landlock syscall-attribute and fd-close invariants, and -has Linux-only regression tests for syscall-free input rejection. The generated -inventory now reports 197 annotated native unsafe sites and 232 remaining -unsafe review sites. +has Linux-only regression tests for syscall-free input rejection. `wasm.rs` +now bounds module/function/path/log buffers, converts poisoned +handle-store locks into normal FFI errors, rejects null argument/result buffers +with nonzero lengths, checks guest-memory offsets with overflow-safe helpers, +and has Rust regressions for invalid C ABI inputs. The generated inventory now +reports 198 annotated native unsafe sites and 229 remaining unsafe review +sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -669,8 +669,12 @@ not started." now validates path and TCP port inputs before issuing syscalls, rejects interior-NUL path truncation, documents Landlock syscall-attribute and fd-close invariants, and has Linux-only regression tests for syscall-free - input rejection. The generated report now shows 197 annotated native unsafe - sites and 232 remaining unsafe review sites. + input rejection. `wasm.rs` now bounds module/function/path/log buffers, + converts poisoned handle-store locks into normal FFI errors, rejects null + argument/result buffers with nonzero lengths, checks guest-memory offsets + with overflow-safe helpers, and has Rust regressions for invalid C ABI + inputs. The generated report now shows 198 annotated native unsafe sites and + 229 remaining unsafe review sites. Remaining work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. --- a/docs/reviews/2026-07-27-native-export-review.sexp +++ b/docs/reviews/2026-07-27-native-export-review.sexp @@ -179,18 +179,18 @@ (export (symbol "jerboa_tls_set_nonblock") (file "jerboa-native-rs/src/tls.rs") (line 1699) (scheme-callers 1 (caller (file "lib/std/net/tls-rustls.ss") (line 150)))) (export (symbol "jerboa_tls_set_timeout") (file "jerboa-native-rs/src/tls.rs") (line 1724) (scheme-callers 1 (caller (file "lib/std/net/tls-rustls.ss") (line 155)))) (export (symbol "jerboa_tls_write") (file "jerboa-native-rs/src/tls.rs") (line 1609) (scheme-callers 1 (caller (file "lib/std/net/tls-rustls.ss") (line 134)))) - (export (symbol "jerboa_wasm_add_fuel") (file "jerboa-native-rs/src/wasm.rs") (line 293) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 118)))) - (export (symbol "jerboa_wasm_call") (file "jerboa-native-rs/src/wasm.rs") (line 341) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 94)))) - (export (symbol "jerboa_wasm_fuel_remaining") (file "jerboa-native-rs/src/wasm.rs") (line 316) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 124)))) - (export (symbol "jerboa_wasm_get_log") (file "jerboa-native-rs/src/wasm.rs") (line 583) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 136)))) - (export (symbol "jerboa_wasm_instance_free") (file "jerboa-native-rs/src/wasm.rs") (line 208) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 88)))) - (export (symbol "jerboa_wasm_instance_new") (file "jerboa-native-rs/src/wasm.rs") (line 154) (scheme-callers 3 (caller (file "lib/std/wasm/sandbox.ss") (line 82)) (caller (file "lib/std/wasm/sandbox.ss") (line 130)) (caller (file "tests/test-wasm-sandbox.ss") (line 500)))) - (export (symbol "jerboa_wasm_instance_new_hosted") (file "jerboa-native-rs/src/wasm.rs") (line 1122) (scheme-callers 2 (caller (file "lib/std/wasm/sandbox.ss") (line 130)) (caller (file "tests/test-wasm-sandbox.ss") (line 500)))) - (export (symbol "jerboa_wasm_memory_read") (file "jerboa-native-rs/src/wasm.rs") (line 466) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 100)))) - (export (symbol "jerboa_wasm_memory_size") (file "jerboa-native-rs/src/wasm.rs") (line 561) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 112)))) - (export (symbol "jerboa_wasm_memory_write") (file "jerboa-native-rs/src/wasm.rs") (line 511) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 106)))) - (export (symbol "jerboa_wasm_module_free") (file "jerboa-native-rs/src/wasm.rs") (line 142) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 76)))) - (export (symbol "jerboa_wasm_module_new") (file "jerboa-native-rs/src/wasm.rs") (line 104) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 70)))) + (export (symbol "jerboa_wasm_add_fuel") (file "jerboa-native-rs/src/wasm.rs") (line 398) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 118)))) + (export (symbol "jerboa_wasm_call") (file "jerboa-native-rs/src/wasm.rs") (line 452) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 94)))) + (export (symbol "jerboa_wasm_fuel_remaining") (file "jerboa-native-rs/src/wasm.rs") (line 424) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 124)))) + (export (symbol "jerboa_wasm_get_log") (file "jerboa-native-rs/src/wasm.rs") (line 734) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 136)))) + (export (symbol "jerboa_wasm_instance_free") (file "jerboa-native-rs/src/wasm.rs") (line 296) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 88)))) + (export (symbol "jerboa_wasm_instance_new") (file "jerboa-native-rs/src/wasm.rs") (line 238) (scheme-callers 3 (caller (file "lib/std/wasm/sandbox.ss") (line 82)) (caller (file "lib/std/wasm/sandbox.ss") (line 130)) (caller (file "tests/test-wasm-sandbox.ss") (line 500)))) + (export (symbol "jerboa_wasm_instance_new_hosted") (file "jerboa-native-rs/src/wasm.rs") (line 1300) (scheme-callers 2 (caller (file "lib/std/wasm/sandbox.ss") (line 130)) (caller (file "tests/test-wasm-sandbox.ss") (line 500)))) + (export (symbol "jerboa_wasm_memory_read") (file "jerboa-native-rs/src/wasm.rs") (line 600) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 100)))) + (export (symbol "jerboa_wasm_memory_size") (file "jerboa-native-rs/src/wasm.rs") (line 709) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 112)))) + (export (symbol "jerboa_wasm_memory_write") (file "jerboa-native-rs/src/wasm.rs") (line 652) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 106)))) + (export (symbol "jerboa_wasm_module_free") (file "jerboa-native-rs/src/wasm.rs") (line 224) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 76)))) + (export (symbol "jerboa_wasm_module_new") (file "jerboa-native-rs/src/wasm.rs") (line 183) (scheme-callers 1 (caller (file "lib/std/wasm/sandbox.ss") (line 70)))) (export (symbol "jerboa_writev2") (file "jerboa-native-rs/src/http_parse.rs") (line 110) (scheme-callers 1 (caller (file "lib/std/net/io.ss") (line 81)))) (export (symbol "jerboa_x509_cert_fingerprint") (file "jerboa-native-rs/src/x509.rs") (line 562) (scheme-callers 1 (caller (file "lib/std/crypto/x509.ss") (line 31)))) (export (symbol "jerboa_x509_generate_self_signed") (file "jerboa-native-rs/src/x509.rs") (line 124) (scheme-callers 1 (caller (file "lib/std/crypto/x509.ss") (line 27)))) --- a/docs/status.md +++ b/docs/status.md @@ -25,7 +25,7 @@ release artifacts are built as Jerboa multicall binaries with `jerboa`, | Area | Current state | Remaining work | |---|---|---| | Kimi security handoff | [kimi3-security-recommmendations.md](kimi3-security-recommmendations.md) is the backlog. Dated evidence and review manifests live under [reviews/](reviews/). | Keep new security evidence in dated review records and summarize the current release state here. | -| FFI audit phase 5 | [ffi-audit.md](ffi-audit.md) records the scanner output, provisional Scheme binding verdicts, and native Rust export inventory. `make native-export-review-check` gates native export decisions. The worker-launch native path in `aproc.rs`/`seccomp.rs`, crypto FFI buffer path in `crypto.rs`, secure-memory region lifecycle in `secure_mem.rs`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, embed-crypto ABI, integrity ABI, regex-native ABI, process-control ABI, pcap ABI, inotify ABI, epoll/eventfd ABI, antidebug ABI, SOCKS5 server ABI, PostgreSQL native ABI, and Landlock native ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 232. | Continue adding `SAFETY:` invariant comments near the remaining Rust unsafe sites. | +| FFI audit phase 5 | [ffi-audit.md](ffi-audit.md) records the scanner output, provisional Scheme binding verdicts, and native Rust export inventory. `make native-export-review-check` gates native export decisions. The worker-launch native path in `aproc.rs`/`seccomp.rs`, crypto FFI buffer path in `crypto.rs`, secure-memory region lifecycle in `secure_mem.rs`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, embed-crypto ABI, integrity ABI, regex-native ABI, process-control ABI, pcap ABI, inotify ABI, epoll/eventfd ABI, antidebug ABI, SOCKS5 server ABI, PostgreSQL native ABI, Landlock native ABI, and wasmi sandbox ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 229. | Continue adding `SAFETY:` invariant comments near the remaining Rust unsafe sites. | | Native Rust exports | The native export review now has 190 exported functions: 183 tracked Scheme references and 7 retained standalone C/binary helpers. The previous 35 no-Scheme-reference removal candidates no longer have C ABI export markers. | Re-run `make native-export-review-check` whenever adding or removing native exports. | | Confined worker | `(std security worker)` provides the facade, audit lifecycle, output caps, deadlines, process-group kill, memory rlimit pre-exec setup, Linux syscall/ptrace seccomp pre-exec setup for requested axes, explicit sandbox-axis refusal, and egress proxy env wiring. | Install native Landlock path/net rules in the worker pre-exec path and keep Linux/macOS/FreeBSD parity tests current. | | Safe surface | Direct scripts default to the safe prelude; raw access requires `--unsafe-prelude` or `(jerboa prelude unsafe)`. | Continue moving risky APIs behind explicit unsafe imports as new modules land. | --- a/jerboa-native-rs/src/wasm.rs +++ b/jerboa-native-rs/src/wasm.rs @@ -12,13 +12,15 @@ //! inside WASM linear memory, an attacker cannot reach Chez runtime gadgets. use std::collections::HashMap; -use std::sync::Mutex; +use std::sync::{Mutex, MutexGuard}; use wasmi::core::ValType; use wasmi::*; use crate::panic::{ffi_wrap, set_last_error}; +const MAX_C_ABI_SLICE_LEN: usize = isize::MAX as usize; + // ============================================================ // Handle management // ============================================================ @@ -93,6 +95,83 @@ fn next_handle() -> u64 { NEXT_WASM_HANDLE.fetch_add(1, std::sync::atomic::Ordering::Relaxed) } +fn lock_modules() -> Option<MutexGuard<'static, HashMap<u64, WasmModule>>> { + match wasm_modules().lock() { + Ok(guard) => Some(guard), + Err(_) => { + set_last_error("WASM module store lock poisoned".to_string()); + None + } + } +} + +fn lock_instances() -> Option<MutexGuard<'static, HashMap<u64, WasmInstance>>> { + match wasm_instances().lock() { + Ok(guard) => Some(guard), + Err(_) => { + set_last_error("WASM instance store lock poisoned".to_string()); + None + } + } +} + +fn ffi_bytes<'a>(ptr: *const u8, len: usize, label: &str) -> Result<&'a [u8], ()> { + if len > MAX_C_ABI_SLICE_LEN { + set_last_error(format!("{label} too large")); + return Err(()); + } + if ptr.is_null() && len != 0 { + set_last_error(format!("null {label}")); + return Err(()); + } + if len == 0 { + return Ok(&[]); + } + // SAFETY: `len` is bounded to `isize::MAX`, and null pointers with + // nonzero length were rejected above. Callers consume the borrowed slice + // entirely within the enclosing C ABI call. + Ok(unsafe { std::slice::from_raw_parts(ptr, len) }) +} + +fn checked_wasm_range(offset: i32, len: i32, memory_len: usize) -> Option<std::ops::Range<usize>> { + if offset < 0 || len < 0 { + return None; + } + let start = offset as usize; + let len = len as usize; + let end = start.checked_add(len)?; + if end > memory_len { + return None; + } + Some(start..end) +} + +fn checked_wasm_range_u32( + offset: u32, + len: u32, + memory_len: usize, +) -> Option<std::ops::Range<usize>> { + let start = offset as usize; + let len = len as usize; + let end = start.checked_add(len)?; + if end > memory_len { + return None; + } + Some(start..end) +} + +fn checked_wasm_range_usize( + offset: usize, + len: usize, + memory_len: usize, +) -> Option<std::ops::Range<usize>> { + let end = offset.checked_add(len)?; + if end > memory_len { + return None; + } + Some(offset..end) +} + // ============================================================ // Module: load and validate WASM bytecode // ============================================================ @@ -103,12 +182,14 @@ fn next_handle() -> u64 { #[no_mangle] pub extern "C" fn jerboa_wasm_module_new(bytes: *const u8, bytes_len: usize) -> u64 { match std::panic::catch_unwind(|| { - if bytes.is_null() || bytes_len == 0 { - set_last_error("null or empty WASM bytecode".to_string()); + if bytes_len == 0 { + set_last_error("empty WASM bytecode".to_string()); return 0; } - - let wasm_bytes = unsafe { std::slice::from_raw_parts(bytes, bytes_len) }; + let wasm_bytes = match ffi_bytes(bytes, bytes_len, "WASM bytecode") { + Ok(value) => value, + Err(()) => return 0, + }; let mut config = Config::default(); config.consume_fuel(true); @@ -123,10 +204,11 @@ pub extern "C" fn jerboa_wasm_module_new(bytes: *const u8, bytes_len: usize) -> }; let handle = next_handle(); - wasm_modules() - .lock() - .unwrap() - .insert(handle, WasmModule { engine, module }); + let mut modules = match lock_modules() { + Some(value) => value, + None => return 0, + }; + modules.insert(handle, WasmModule { engine, module }); handle }) { Ok(h) => h, @@ -140,7 +222,9 @@ pub extern "C" fn jerboa_wasm_module_new(bytes: *const u8, bytes_len: usize) -> /// Free a WASM module. #[no_mangle] pub extern "C" fn jerboa_wasm_module_free(handle: u64) { - let _ = wasm_modules().lock().unwrap().remove(&handle); + if let Some(mut modules) = lock_modules() { + let _ = modules.remove(&handle); + } } // ============================================================ @@ -153,7 +237,10 @@ pub extern "C" fn jerboa_wasm_module_free(handle: u64) { #[no_mangle] pub extern "C" fn jerboa_wasm_instance_new(module_handle: u64, fuel: u64) -> u64 { match std::panic::catch_unwind(|| { - let modules = wasm_modules().lock().unwrap(); + let modules = match lock_modules() { + Some(value) => value, + None => return 0, + }; let wmod = match modules.get(&module_handle) { Some(m) => m, None => { @@ -189,10 +276,11 @@ pub extern "C" fn jerboa_wasm_instance_new(module_handle: u64, fuel: u64) -> u64 }; let handle = next_handle(); - wasm_instances() - .lock() - .unwrap() - .insert(handle, WasmInstance { store, instance }); + let mut instances = match lock_instances() { + Some(value) => value, + None => return 0, + }; + instances.insert(handle, WasmInstance { store, instance }); handle }) { Ok(h) => h, @@ -206,7 +294,9 @@ pub extern "C" fn jerboa_wasm_instance_new(module_handle: u64, fuel: u64) -> u64 /// Free a WASM instance. #[no_mangle] pub extern "C" fn jerboa_wasm_instance_free(handle: u64) { - let _ = wasm_instances().lock().unwrap().remove(&handle); + if let Some(mut instances) = lock_instances() { + let _ = instances.remove(&handle); + } } /// Attach a pre-opened UDP socket fd to a hosted WASM instance. @@ -218,7 +308,10 @@ pub extern "C" fn jerboa_wasm_instance_free(handle: u64) { /// Returns 0 on success, -1 on error. pub extern "C" fn jerboa_wasm_set_socket(instance_handle: u64, fd: i32) -> i32 { ffi_wrap(|| { - let mut instances = wasm_instances().lock().unwrap(); + let mut instances = match lock_instances() { + Some(value) => value, + None => return -1, + }; let inst = match instances.get_mut(&instance_handle) { Some(i) => i, None => { @@ -228,8 +321,14 @@ pub extern "C" fn jerboa_wasm_set_socket(instance_handle: u64, fd: i32) -> i32 { }; #[cfg(unix)] { + if fd < 0 { + set_last_error("invalid WASM UDP socket fd".to_string()); + return -1; + } use std::os::unix::io::FromRawFd; - // SAFETY: caller must ensure fd is a valid, owned UDP socket fd. + // SAFETY: caller must ensure fd is a valid, owned UDP socket fd; + // this wrapper rejects negative fds and takes ownership exactly + // once by storing the resulting UdpSocket in the instance state. let socket = unsafe { std::net::UdpSocket::from_raw_fd(fd) }; inst.store.data_mut().udp_socket = Some(socket); inst.store.data_mut().peer_addr = None; @@ -254,11 +353,14 @@ pub extern "C" fn jerboa_wasm_allow_cdb_dir( path_len: usize, ) -> i32 { ffi_wrap(|| { - if path_ptr.is_null() { - set_last_error("null path pointer".to_string()); + if path_len == 0 { + set_last_error("empty CDB directory path".to_string()); return -1; } - let path_bytes = unsafe { std::slice::from_raw_parts(path_ptr, path_len) }; + let path_bytes = match ffi_bytes(path_ptr, path_len, "CDB directory path") { + Ok(value) => value, + Err(()) => return -1, + }; let path_str = match std::str::from_utf8(path_bytes) { Ok(s) => s, Err(_) => { @@ -274,7 +376,10 @@ pub extern "C" fn jerboa_wasm_allow_cdb_dir( return -1; } }; - let mut instances = wasm_instances().lock().unwrap(); + let mut instances = match lock_instances() { + Some(value) => value, + None => return -1, + }; let inst = match instances.get_mut(&instance_handle) { Some(i) => i, None => { @@ -292,7 +397,10 @@ pub extern "C" fn jerboa_wasm_allow_cdb_dir( #[no_mangle] pub extern "C" fn jerboa_wasm_add_fuel(handle: u64, fuel: u64) -> i32 { ffi_wrap(|| { - let mut instances = wasm_instances().lock().unwrap(); + let mut instances = match lock_instances() { + Some(value) => value, + None => return -1, + }; let inst = match instances.get_mut(&handle) { Some(i) => i, None => { @@ -315,7 +423,10 @@ pub extern "C" fn jerboa_wasm_add_fuel(handle: u64, fuel: u64) -> i32 { #[no_mangle] pub extern "C" fn jerboa_wasm_fuel_remaining(handle: u64) -> i64 { match std::panic::catch_unwind(|| { - let instances = wasm_instances().lock().unwrap(); + let instances = match lock_instances() { + Some(value) => value, + None => return -1i64, + }; let inst = match instances.get(&handle) { Some(i) => i, None => return -1i64, @@ -348,21 +459,22 @@ pub extern "C" fn jerboa_wasm_call( nresults: usize, ) -> i32 { match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - if name.is_null() { - set_last_error("null function name".to_string()); - return -1; - } - - let func_name = - match std::str::from_utf8(unsafe { std::slice::from_raw_parts(name, name_len) }) { - Ok(s) => s, - Err(_) => { - set_last_error("invalid UTF-8 function name".to_string()); - return -1; - } - }; + let name_bytes = match ffi_bytes(name, name_len, "WASM function name") { + Ok(value) => value, + Err(()) => return -1, + }; + let func_name = match std::str::from_utf8(name_bytes) { + Ok(s) => s, + Err(_) => { + set_last_error("invalid UTF-8 function name".to_string()); + return -1; + } + }; - let mut instances = wasm_instances().lock().unwrap(); + let mut instances = match lock_instances() { + Some(value) => value, + None => return -1, + }; let inst = match instances.get_mut(&handle) { Some(i) => i, None => { @@ -393,6 +505,13 @@ pub extern "C" fn jerboa_wasm_call( } let wasm_args: Vec<Val> = if nargs > 0 && !args.is_null() { + if nargs > MAX_C_ABI_SLICE_LEN / std::mem::size_of::<i64>() { + set_last_error("WASM argument array too large".to_string()); + return -1; + } + // SAFETY: `args` is non-null, the element count is bounded to keep + // the byte length within `isize::MAX`, and the slice is consumed + // before this C ABI call returns. let arg_slice = unsafe { std::slice::from_raw_parts(args, nargs) }; arg_slice .iter() @@ -406,6 +525,10 @@ pub extern "C" fn jerboa_wasm_call( }) .collect() } else { + if nargs > 0 { + set_last_error("null WASM argument array".to_string()); + return -1; + } vec![] }; @@ -430,7 +553,18 @@ pub extern "C" fn jerboa_wasm_call( } // Copy results out - if !results.is_null() && nresults > 0 { + if nresults > 0 { + if results.is_null() { + set_last_error("null WASM results array".to_string()); + return -1; + } + if nresults > MAX_C_ABI_SLICE_LEN / std::mem::size_of::<i64>() { + set_last_error("WASM results array too large".to_string()); + return -1; + } + // SAFETY: `results` is non-null, the element count is bounded to + // keep the byte length within `isize::MAX`, and writes are capped + // to the caller-provided result slot count. let out = unsafe { std::slice::from_raw_parts_mut(results, nresults) }; for (i, val) in wasm_results.into_iter().enumerate() { if i >= nresults { @@ -465,12 +599,15 @@ pub extern "C" fn jerboa_wasm_call( #[no_mangle] pub extern "C" fn jerboa_wasm_memory_read(handle: u64, offset: u32, buf: *mut u8, len: u32) -> i32 { ffi_wrap(|| { - if buf.is_null() { + if buf.is_null() && len != 0 { set_last_error("null buffer".to_string()); return -1; } - let instances = wasm_instances().lock().unwrap(); + let instances = match lock_instances() { + Some(value) => value, + None => return -1, + }; let inst = match instances.get(&handle) { Some(i) => i, None => { @@ -488,19 +625,23 @@ pub extern "C" fn jerboa_wasm_memory_read(handle: u64, offset: u32, buf: *mut u8 }; let mem_data = memory.data(&inst.store); - let start = offset as usize; - let end = start + len as usize; + let range = match checked_wasm_range_u32(offset, len, mem_data.len()) { + Some(value) => value, + None => { + set_last_error(format!( + "memory read OOB: offset={offset} len={len} size={}", + mem_data.len() + )); + return -1; + } + }; - if end > mem_data.len() { - set_last_error(format!( - "memory read OOB: offset={offset} len={len} size={}", - mem_data.len() - )); - return -1; + if len != 0 { + // SAFETY: null buffers with nonzero length were rejected, and + // `len` is a u32, keeping the C ABI slice below `isize::MAX`. + let out = unsafe { std::slice::from_raw_parts_mut(buf, len as usize) }; + out.copy_from_slice(&mem_data[range]); } - - let out = unsafe { std::slice::from_raw_parts_mut(buf, len as usize) }; - out.copy_from_slice(&mem_data[start..end]); len as i32 }) } @@ -515,12 +656,15 @@ pub extern "C" fn jerboa_wasm_memory_write( len: u32, ) -> i32 { ffi_wrap(|| { - if buf.is_null() { + if buf.is_null() && len != 0 { set_last_error("null buffer".to_string()); return -1; } - let mut instances = wasm_instances().lock().unwrap(); + let mut instances = match lock_instances() { + Some(value) => value, + None => return -1, + }; let inst = match instances.get_mut(&handle) { Some(i) => i, None => { @@ -538,19 +682,23 @@ pub extern "C" fn jerboa_wasm_memory_write( }; let mem_data = memory.data_mut(&mut inst.store); - let start = offset as usize; - let end = start + len as usize; + let range = match checked_wasm_range_u32(offset, len, mem_data.len()) { + Some(value) => value, + None => { + set_last_error(format!( + "memory write OOB: offset={offset} len={len} size={}", + mem_data.len() + )); + return -1; + } + }; - if end > mem_data.len() { - set_last_error(format!( - "memory write OOB: offset={offset} len={len} size={}", - mem_data.len() - )); - return -1; + if len != 0 { + // SAFETY: null buffers with nonzero length were rejected, and + // `len` is a u32, keeping the C ABI slice below `isize::MAX`. + let input = unsafe { std::slice::from_raw_parts(buf, len as usize) }; + mem_data[range].copy_from_slice(input); } - - let input = unsafe { std::slice::from_raw_parts(buf, len as usize) }; - mem_data[start..end].copy_from_slice(input); 0 }) } @@ -560,7 +708,10 @@ pub extern "C" fn jerboa_wasm_memory_write( #[no_mangle] pub extern "C" fn jerboa_wasm_memory_size(handle: u64) -> i64 { match std::panic::catch_unwind(|| { - let instances = wasm_instances().lock().unwrap(); + let instances = match lock_instances() { + Some(value) => value, + None => return -1i64, + }; let inst = match instances.get(&handle) { Some(i) => i, None => return -1i64, @@ -582,7 +733,18 @@ pub extern "C" fn jerboa_wasm_memory_size(handle: u64) -> i64 { #[no_mangle] pub extern "C" fn jerboa_wasm_get_log(handle: u64, buf_ptr: *mut u8, buf_max: usize) -> i64 { match std::panic::catch_unwind(|| { - let instances = wasm_instances().lock().unwrap(); + if buf_ptr.is_null() && buf_max != 0 { + set_last_error("null WASM log output".to_string()); + return -1i64; + } + if buf_max > MAX_C_ABI_SLICE_LEN { + set_last_error("WASM log output buffer too large".to_string()); + return -1i64; + } + let instances = match lock_instances() { + Some(value) => value, + None => return -1i64, + }; let inst = match instances.get(&handle) { Some(i) => i, None => return -1i64, @@ -592,6 +754,9 @@ pub extern "C" fn jerboa_wasm_get_log(handle: u64, buf_ptr: *mut u8, buf_max: us let bytes = full.as_bytes(); if !buf_ptr.is_null() && buf_max > 0 { let copy_len = bytes.len().min(buf_max); + // SAFETY: `buf_ptr` is non-null for a nonzero buffer, `buf_max` + // is bounded to `isize::MAX`, and `copy_len` is capped to both + // source and destination lengths. unsafe { std::ptr::copy_nonoverlapping(bytes.as_ptr(), buf_ptr, copy_len); } @@ -697,24 +862,34 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { if fd != 1 && fd != 2 { return 8; } + if iovs_len < 0 { + return 28; + } let mut total = 0u32; for i in 0..iovs_len { - let iov_addr = (iovs_ptr + i * 8) as usize; + let iov_addr = match i + .checked_mul(8) + .and_then(|delta| iovs_ptr.checked_add(delta)) + { + Some(value) => value, + None => return 21, + }; let mem_data = memory.data(&caller); - if iov_addr + 8 > mem_data.len() { - return 21; - } - let buf_ptr = - u32::from_le_bytes(mem_data[iov_addr..iov_addr + 4].try_into().unwrap()); - let buf_len = - u32::from_le_bytes(mem_data[iov_addr + 4..iov_addr + 8].try_into().unwrap()); + let iov_range = match checked_wasm_range(iov_addr, 8, mem_data.len()) { + Some(value) => value, + None => return 21, + }; + let iov = &mem_data[iov_range]; + let buf_ptr = u32::from_le_bytes(iov[0..4].try_into().unwrap()); + let buf_len = u32::from_le_bytes(iov[4..8].try_into().unwrap()); let start = buf_ptr as usize; - let end = start + buf_len as usize; let mem_data2 = memory.data(&caller); - if end > mem_data2.len() { - return 21; - } - let bytes = mem_data2[start..end].to_vec(); + let data_range = + match checked_wasm_range_usize(start, buf_len as usize, mem_data2.len()) { + Some(value) => value, + None => return 21, + }; + let bytes = mem_data2[data_range].to_vec(); if fd == 1 { let _ = std::io::Write::write_all(&mut std::io::stdout(), &bytes); } else { @@ -749,17 +924,26 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { if fd != 0 { return 8; } + if iovs_len < 0 { + return 28; + } let mut total = 0u32; for i in 0..iovs_len { - let iov_addr = (iovs_ptr + i * 8) as usize; + let iov_addr = match i + .checked_mul(8) + .and_then(|delta| iovs_ptr.checked_add(delta)) + { + Some(value) => value, + None => return 21, + }; let mem_data = memory.data(&caller); - if iov_addr + 8 > mem_data.len() { - return 21; - } - let buf_ptr = - u32::from_le_bytes(mem_data[iov_addr..iov_addr + 4].try_into().unwrap()); - let buf_len = - u32::from_le_bytes(mem_data[iov_addr + 4..iov_addr + 8].try_into().unwrap()); + let iov_range = match checked_wasm_range(iov_addr, 8, mem_data.len()) { + Some(value) => value, + None => return 21, + }; + let iov = &mem_data[iov_range]; + let buf_ptr = u32::from_le_bytes(iov[0..4].try_into().unwrap()); + let buf_len = u32::from_le_bytes(iov[4..8].try_into().unwrap()); let mut buf = vec![0u8; buf_len as usize]; let n = match std::io::Read::read(&mut std::io::stdin(), &mut buf) { Ok(n) => n, @@ -767,9 +951,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { }; let mem_data_mut = memory.data_mut(&mut caller); let start = buf_ptr as usize; - if start + n <= mem_data_mut.len() { - mem_data_mut[start..start + n].copy_from_slice(&buf[..n]); - } + let range = match checked_wasm_range_usize(start, n, mem_data_mut.len()) { + Some(value) => value, + None => return 21, + }; + mem_data_mut[range].copy_from_slice(&buf[..n]); total += n as u32; if n < buf_len as usize { break; @@ -822,12 +1008,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { _ => return 8, }; let mem_data = memory.data_mut(&mut caller); - let start = buf_ptr as usize; - let end = start + buf_len.max(0) as usize; - if end > mem_data.len() { - return 21; - } - match getrandom::getrandom(&mut mem_data[start..end]) { + let range = match checked_wasm_range(buf_ptr, buf_len, mem_data.len()) { + Some(value) => value, + None => return 21, + }; + match getrandom::getrandom(&mut mem_data[range]) { Ok(()) => 0, Err(_) => 29, // ENOSYS — fall back to caller handling } @@ -855,12 +1040,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { }; let msg = { let data = memory.data(&caller); - let start = msg_ptr as usize; - let end = start + msg_len as usize; - if end > data.len() { - return -1; - } - String::from_utf8_lossy(&data[start..end]).to_string() + let range = match checked_wasm_range(msg_ptr, msg_len, data.len()) { + Some(value) => value, + None => return -1, + }; + String::from_utf8_lossy(&data[range]).to_string() }; let lvl = match level { 0 => "ERROR", @@ -913,11 +1097,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { Ok((n, addr)) => { caller.data_mut().peer_addr = Some(addr); let mem = memory.data_mut(&mut caller); - let start = buf_ptr as usize; - if start + n > mem.len() { - return -1; - } - mem[start..start + n].copy_from_slice(&tmp[..n]); + let range = match checked_wasm_range(buf_ptr, n as i32, mem.len()) { + Some(value) => value, + None => return -1, + }; + mem[range].copy_from_slice(&tmp[..n]); n as i32 } Err(_) => -1, @@ -944,25 +1128,21 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { // Copy packet bytes out of WASM memory. let packet = { let data = memory.data(&caller); - let start = buf_ptr as usize; - let end = start + buf_len.max(0) as usize; - if end > data.len() { - return -1; - } - data[start..end].to_vec() + let range = match checked_wasm_range(buf_ptr, buf_len, data.len()) { + Some(value) => value, + None => return -1, + }; + data[range].to_vec() }; // Optionally parse destination address from WASM memory. let target: Option<std::net::SocketAddr> = if addr_len > 0 { let addr_str = { let data = memory.data(&caller); - let start = addr_ptr as usize; - let end = start + addr_len as usize; - if end > data.len() { - return -1; - } - std::str::from_utf8(&data[start..end]) - .ok() - .map(str::to_string) + let range = match checked_wasm_range(addr_ptr, addr_len, data.len()) { + Some(value) => value, + None => return -1, + }; + std::str::from_utf8(&data[range]).ok().map(str::to_string) }; // If explicit address was provided but can't be parsed, fail // (don't silently fall back to peer_addr) @@ -1006,12 +1186,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { }; let path = { let data = memory.data(&caller); - let start = path_ptr as usize; - let end = start + path_len.max(0) as usize; - if end > data.len() { - return -1; - } - match std::str::from_utf8(&data[start..end]) { + let range = match checked_wasm_range(path_ptr, path_len, data.len()) { + Some(value) => value, + None => return -1, + }; + match std::str::from_utf8(&data[range]) { Ok(s) => s.to_string(), Err(_) => return -1, } @@ -1071,12 +1250,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { }; let key = { let data = memory.data(&caller); - let start = key_ptr as usize; - let end = start + key_len.max(0) as usize; - if end > data.len() { - return -1; - } - data[start..end].to_vec() + let range = match checked_wasm_range(key_ptr, key_len, data.len()) { + Some(value) => value, + None => return -1, + }; + data[range].to_vec() }; let val = { let state = caller.data(); @@ -1090,11 +1268,11 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { Some(v) => { let n = v.len().min(val_max.max(0) as usize); let mem = memory.data_mut(&mut caller); - let start = val_buf as usize; - if start + n > mem.len() { - return -1; - } - mem[start..start + n].copy_from_slice(&v[..n]); + let range = match checked_wasm_range(val_buf, n as i32, mem.len()) { + Some(value) => value, + None => return -1, + }; + mem[range].copy_from_slice(&v[..n]); n as i32 } } @@ -1121,7 +1299,10 @@ fn define_host_imports(linker: &mut Linker<HostState>) -> Result<(), Error> { #[no_mangle] pub extern "C" fn jerboa_wasm_instance_new_hosted(module_handle: u64, fuel: u64) -> u64 { match std::panic::catch_unwind(|| { - let modules = wasm_modules().lock().unwrap(); + let modules = match lock_modules() { + Some(value) => value, + None => return 0, + }; let wmod = match modules.get(&module_handle) { Some(m) => m, None => { @@ -1161,10 +1342,11 @@ pub extern "C" fn jerboa_wasm_instance_new_hosted(module_handle: u64, fuel: u64) }; let handle = next_handle(); - wasm_instances() - .lock() - .unwrap() - .insert(handle, WasmInstance { store, instance }); + let mut instances = match lock_instances() { + Some(value) => value, + None => return 0, + }; + instances.insert(handle, WasmInstance { store, instance }); handle }) { Ok(h) => h, @@ -1174,3 +1356,50 @@ pub extern "C" fn jerboa_wasm_instance_new_hosted(module_handle: u64, fuel: u64) } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ffi_bytes_rejects_null_with_nonzero_length() { + assert!(ffi_bytes(std::ptr::null(), 1, "test bytes").is_err()); + assert_eq!(ffi_bytes(std::ptr::null(), 0, "test bytes").unwrap(), b""); + } + + #[test] + fn wasm_ranges_reject_negative_and_overflow() { + assert!(checked_wasm_range(-1, 0, 10).is_none()); + assert!(checked_wasm_range(0, -1, 10).is_none()); + assert!(checked_wasm_range(i32::MAX, 1, 10).is_none()); + assert_eq!(checked_wasm_range(2, 3, 10).unwrap(), 2..5); + assert!(checked_wasm_range_usize(usize::MAX, 1, usize::MAX).is_none()); + } + + #[test] + fn public_ffi_rejects_invalid_buffers_before_handles() { + assert_eq!(jerboa_wasm_module_new(std::ptr::null(), 1), 0); + assert_eq!(jerboa_wasm_allow_cdb_dir(u64::MAX, std::ptr::null(), 1), -1); + assert_eq!( + jerboa_wasm_call( + u64::MAX, + std::ptr::null(), + 1, + std::ptr::null(), + 0, + std::ptr::null_mut(), + 0 + ), + -1 + ); + assert_eq!( + jerboa_wasm_memory_read(u64::MAX, 0, std::ptr::null_mut(), 1), + -1 + ); + assert_eq!( + jerboa_wasm_memory_write(u64::MAX, 0, std::ptr::null(), 1), + -1 + ); + assert_eq!(jerboa_wasm_get_log(u64::MAX, std::ptr::null_mut(), 1), -1); + } +}