security: annotate process control ffi
ober
c5f63d464ce629e825fbaa3122d12e357b2f8d12
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -41,9 +41,9 @@ Latest summary from `tools/ffi-audit-report.ss`: (c-file-count 5) (no-mangle-exports 196) (unsafe-sites 432) - (unsafe-sites-with-nearby-safety-comment 151) - (unsafe-sites-without-nearby-safety-comment 281) - (unsafe-comment-review-sites 281) + (unsafe-sites-with-nearby-safety-comment 162) + (unsafe-sites-without-nearby-safety-comment 270) + (unsafe-comment-review-sites 270) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -98,9 +98,12 @@ arithmetic before slicing, and streams caller-sized file regions instead of allocating a caller-controlled `u64` length. `regex_native.rs` now validates nullable byte buffers through a shared helper before creating Rust slices, documents output-slot and ovector invariants, checks capture slot arithmetic, -and returns a normal FFI error if the regex handle store is poisoned. The -generated inventory now reports 151 annotated native unsafe sites and 281 -remaining unsafe review sites. +and returns a normal FFI error if the regex handle store is poisoned. +`process_ctl.rs` now documents process-name, memory-lock, signal-probe, errno, +FreeBSD sysctl/setproctitle, and executable-path output invariants, and rejects +FreeBSD process-title capacity overflow before allocating. The generated +inventory now reports 162 annotated native unsafe sites and 270 remaining unsafe +review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -642,8 +642,11 @@ not started." allocating a caller-controlled `u64` length. `regex_native.rs` now validates nullable byte buffers through a shared helper before creating Rust slices, documents output-slot and ovector invariants, checks capture slot arithmetic, - and returns a normal FFI error if the regex handle store is poisoned. The - generated report now shows 151 annotated native unsafe sites and 281 remaining + and returns a normal FFI error if the regex handle store is poisoned. + `process_ctl.rs` now documents process-name, memory-lock, signal-probe, + errno, FreeBSD sysctl/setproctitle, and executable-path output invariants, and + rejects FreeBSD process-title capacity overflow before allocating. The + generated report now shows 162 annotated native unsafe sites and 270 remaining unsafe review sites. Remaining work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. @@ -1133,7 +1136,7 @@ Track these in `docs/status.md` per release: | Confined worker exists | facade, audit lifecycle, output caps, deadline, process-group kill, memory rlimit pre-exec path, explicit sandbox-axis refusal, egress proxy env wiring, and Linux syscall/ptrace seccomp pre-exec setup landed; Landlock path/net and Seatbelt/Capsicum parity pending | yes, tested (P0-02) | | Unclassified raw `read`/FASL/`load` sites | scanner-driven manifest classification gate closed 2026-07-27; trusted build, compiler, REPL/dev, and test surfaces are centrally justified by exact source line | 0 | | FFI bindings audited | scanner inventory and provisional verdicts in [ffi-audit.md](ffi-audit.md); native exports gated and justified | 100% with final verdicts | -| Un-annotated Rust `unsafe` blocks | 281 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 270 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | | Fuzz corpora / crash regressions | 0 / 0 | per-parser corpora + every crash a test | | Exploit-shaped regression tests | ~0 | ≥ 1 per historical finding | | Sandboxed-by-default app entry | no | yes (P0-06) | --- 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, and regex-native ABI now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 281. | 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, and process-control ABI now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 270. | 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/process_ctl.rs +++ b/jerboa-native-rs/src/process_ctl.rs @@ -12,8 +12,12 @@ pub extern "C" fn jerboa_prctl_set_name(name: *const u8, name_len: usize) -> i32 { let len = name_len.min(15); let mut buf = [0u8; 16]; // 15 chars + null + // SAFETY: name is non-null above, len is bounded to the local + // 15-byte process-name buffer, and buf has 16 bytes for the NUL. unsafe { std::ptr::copy_nonoverlapping(name, buf.as_mut_ptr(), len) }; buf[len] = 0; + // SAFETY: PR_SET_NAME expects a pointer to a NUL-terminated + // 16-byte-or-smaller thread name cast through c_ulong on Linux. let rc = unsafe { libc::prctl(libc::PR_SET_NAME, buf.as_ptr() as libc::c_ulong, 0, 0, 0) }; if rc != 0 { @@ -36,6 +40,8 @@ pub extern "C" fn jerboa_mlockall() -> i32 { ffi_wrap(|| { #[cfg(any(target_os = "linux", target_os = "freebsd"))] { + // SAFETY: mlockall has no pointer arguments; flags are the + // documented libc constants for current and future mappings. let rc = unsafe { libc::mlockall(libc::MCL_CURRENT | libc::MCL_FUTURE) }; if rc != 0 { set_last_error("mlockall failed (may need root)".into()); @@ -55,6 +61,8 @@ pub extern "C" fn jerboa_mlockall() -> i32 { /// Returns: 1 = exists, 0 = does not exist, -1 = error. pub extern "C" fn jerboa_kill_probe(pid: u32) -> i32 { ffi_wrap(|| { + // SAFETY: kill(pid, 0) performs permission/existence probing without + // delivering a signal; pid_t conversion is the platform libc ABI. let rc = unsafe { libc::kill(pid as libc::pid_t, 0) }; if rc == 0 { 1 // process exists @@ -75,11 +83,15 @@ pub extern "C" fn jerboa_kill_probe(pid: u32) -> i32 { /// Cross-platform errno accessor #[cfg(target_os = "linux")] fn errno() -> i32 { + // SAFETY: __errno_location returns the calling thread's errno slot on + // Linux; immediately reading the pointed-to int is the libc contract. unsafe { *libc::__errno_location() } } #[cfg(target_os = "freebsd")] fn errno() -> i32 { + // SAFETY: __error returns the calling thread's errno slot on FreeBSD; + // immediately reading the pointed-to int is the libc contract. unsafe { *libc::__error() } } @@ -95,11 +107,18 @@ pub extern "C" fn jerboa_freebsd_is_traced() -> i32 { ffi_wrap(|| { #[cfg(target_os = "freebsd")] { + // SAFETY: getpid has no pointer arguments and returns the current + // process id for the sysctl MIB. let pid = unsafe { libc::getpid() }; let mut mib: [libc::c_int; 4] = [libc::CTL_KERN, libc::KERN_PROC, libc::KERN_PROC_PID, pid]; + // SAFETY: kinfo_proc is a plain libc output struct; it is + // immediately filled by sysctl before fields are inspected. let mut kinfo: libc::kinfo_proc = unsafe { std::mem::zeroed() }; let mut len = std::mem::size_of::<libc::kinfo_proc>(); + // SAFETY: sysctl receives a fixed MIB, a writable kinfo_proc + // output buffer and length slot, and null new-value pointers for a + // read-only query. let rc = unsafe { libc::sysctl( mib.as_mut_ptr(), @@ -138,6 +157,8 @@ pub extern "C" fn jerboa_freebsd_process_count() -> i32 { let mut mib: [libc::c_int; 3] = [libc::CTL_KERN, libc::KERN_PROC, libc::KERN_PROC_ALL]; let mut len: usize = 0; // First call: get required buffer size + // SAFETY: this sysctl size query passes a fixed MIB, null output + // buffer, and a writable length slot, with no new value. let rc = unsafe { libc::sysctl( mib.as_mut_ptr(), @@ -179,8 +200,17 @@ pub extern "C" fn jerboa_setproctitle(name: *const u8, name_len: usize) -> i32 { #[cfg(target_os = "freebsd")] { // Build a null-terminated C string + let capacity = match name_len.checked_add(1) { + Some(n) => n, + None => { + set_last_error("process title length overflow".into()); + return -1; + } + }; + // SAFETY: name is non-null above and the C ABI contract requires a + // readable name_len-byte process title buffer. let slice = unsafe { std::slice::from_raw_parts(name, name_len) }; - let mut buf = Vec::with_capacity(name_len + 1); + let mut buf = Vec::with_capacity(capacity); buf.extend_from_slice(slice); buf.push(0); // setproctitle(fmt, ...) is varargs. We use "%s" format to avoid @@ -188,6 +218,9 @@ pub extern "C" fn jerboa_setproctitle(name: *const u8, name_len: usize) -> i32 { extern "C" { fn setproctitle(fmt: *const libc::c_char, ...); } + // SAFETY: setproctitle is called with a constant "%s" format and a + // NUL-terminated byte buffer, so caller bytes are not interpreted as + // a format string. unsafe { setproctitle( b"%s\0".as_ptr() as *const libc::c_char, @@ -226,6 +259,8 @@ pub extern "C" fn jerboa_proc_self_exe( let bytes = path.to_string_lossy(); let path_bytes = bytes.as_bytes(); let copy_len = path_bytes.len().min(output_len); + // SAFETY: output and actual_len are non-null above; copy_len + // is bounded by output_len and path_bytes.len(). unsafe { std::ptr::copy_nonoverlapping(path_bytes.as_ptr(), output, copy_len); *actual_len = path_bytes.len(); @@ -249,6 +284,9 @@ pub extern "C" fn jerboa_proc_self_exe( ]; let mut buf = vec![0u8; 4096]; let mut len = buf.len(); + // SAFETY: sysctl receives a fixed KERN_PROC_PATHNAME MIB, a + // writable pathname buffer and length slot, and null new-value + // pointers for a read-only query. let rc = unsafe { libc::sysctl( mib.as_mut_ptr(), @@ -270,6 +308,8 @@ pub extern "C" fn jerboa_proc_self_exe( len }; let copy_len = path_len.min(output_len); + // SAFETY: output and actual_len are non-null above; copy_len is + // bounded by output_len and the initialized sysctl buffer length. unsafe { std::ptr::copy_nonoverlapping(buf.as_ptr(), output, copy_len); *actual_len = path_len; @@ -285,3 +325,28 @@ pub extern "C" fn jerboa_proc_self_exe( } }) } + +#[cfg(test)] +mod tests { + use super::{jerboa_prctl_set_name, jerboa_proc_self_exe, jerboa_setproctitle}; + + #[test] + fn set_name_rejects_null_name() { + assert_eq!(jerboa_prctl_set_name(std::ptr::null(), 1), -1); + } + + #[test] + fn setproctitle_rejects_null_name() { + assert_eq!(jerboa_setproctitle(std::ptr::null(), 1), -1); + } + + #[test] + fn proc_self_exe_rejects_null_actual_len() { + let mut output = [0u8; 16]; + + assert_eq!( + jerboa_proc_self_exe(output.as_mut_ptr(), output.len(), std::ptr::null_mut()), + -1 + ); + } +}