security: document inotify process socks unsafe invariants
ober
90c980a404c571947fd34bbc0cde61308f6c28ce
--- 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 369) - (unsafe-sites-with-nearby-safety-comment 335) - (unsafe-sites-without-nearby-safety-comment 34) - (unsafe-comment-review-sites 34) + (unsafe-sites-with-nearby-safety-comment 347) + (unsafe-sites-without-nearby-safety-comment 22) + (unsafe-comment-review-sites 22) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -100,8 +100,9 @@ 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. `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. `pcap_capture.rs` +FreeBSD sysctl/setproctitle, and executable-path output invariants adjacent to +its remaining unsafe sites, and rejects FreeBSD process-title capacity overflow +before allocating. `pcap_capture.rs` now routes its C ABI exports through panic containment, documents BPF ioctl, packet-buffer, timestamp, interface-list, and `getifaddrs` invariants, and has device-free regressions for null pointer rejection. `panic.rs` now has an @@ -117,7 +118,8 @@ reports ptrace OS errors, and saturates timing duration conversion instead of truncating. `socks5_server.rs` now rejects null pointers paired with nonzero lengths before building FFI slices, bounds caller lengths, documents stats buffer copies, and returns normal FFI errors for poisoned server-registry -locks. `postgres_native.rs` now funnels SQL and connection-string inputs +locks, with C ABI slice and output-copy invariants adjacent to the audited +unsafe expressions. `postgres_native.rs` now funnels SQL and connection-string inputs through a shared checked UTF-8 helper, rejects null/nonzero and oversized FFI buffers, documents result-slot and output-buffer writes, and permits zero-sized output buffers without manufacturing null slices. `landlock.rs` now validates @@ -159,7 +161,7 @@ funnels hostname, path, PEM, pin, and read/write buffers through checked C ABI helpers, rejects null/nonzero and oversized buffers before slicing, rejects negative accepted fds before adopting them as `TcpStream`, and documents socket shutdown invariants used to break blocked TLS I/O. The generated inventory now -reports 335 annotated native unsafe sites and 34 remaining unsafe review sites. +reports 347 annotated native unsafe sites and 22 remaining unsafe review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -650,8 +650,9 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit documents output-slot and ovector invariants, checks capture slot arithmetic, 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. + errno, FreeBSD sysctl/setproctitle, and executable-path output invariants + adjacent to its remaining unsafe sites, and rejects FreeBSD process-title + capacity overflow before allocating. `pcap_capture.rs` now routes its C ABI exports through panic containment, documents BPF ioctl, packet-buffer, timestamp, interface-list, and `getifaddrs` invariants, and has device-free regressions for null pointer @@ -668,7 +669,8 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit truncating. `socks5_server.rs` now rejects null pointers paired with nonzero lengths before building FFI slices, bounds caller lengths, documents stats buffer copies, and returns normal FFI errors for poisoned server-registry - locks. `postgres_native.rs` now funnels SQL and connection-string inputs + locks, with C ABI slice and output-copy invariants adjacent to the audited + unsafe expressions. `postgres_native.rs` now funnels SQL and connection-string inputs through a shared checked UTF-8 helper, rejects null/nonzero and oversized FFI buffers, documents result-slot and output-buffer writes, and permits zero-sized output buffers without manufacturing null slices. `landlock.rs` @@ -700,8 +702,8 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit publication, and test-owned fd conversion invariants. `tls.rs` now validates hostname, path, PEM, pin, and read/write buffers through shared helpers, rejects invalid accepted fds before ownership transfer, and documents socket - shutdown invariants for blocked TLS I/O. The generated report now shows 335 - annotated native unsafe sites and 34 remaining unsafe review sites. + shutdown invariants for blocked TLS I/O. The generated report now shows 347 + annotated native unsafe sites and 22 remaining unsafe review sites. Remaining work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. @@ -1191,7 +1193,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 | 34 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 22 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/reviews/2026-07-27-native-export-review.sexp +++ b/docs/reviews/2026-07-27-native-export-review.sexp @@ -68,10 +68,10 @@ (export (symbol "jerboa_http_parse") (file "jerboa-native-rs/src/http_parse.rs") (line 30) (scheme-callers 4 (caller (file "lib/std/net/fiber-httpd.ss") (line 78)) (caller (file "lib/std/net/fiber-httpd.ss") (line 81)) (caller (file "lib/std/net/httpsd.ss") (line 28)) (caller (file "lib/std/net/httpsd.ss") (line 71)))) (export (symbol "jerboa_inflate") (file "jerboa-native-rs/src/compress.rs") (line 55) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 33)))) (export (symbol "jerboa_inotify_add_watch") (file "jerboa-native-rs/src/inotify_native.rs") (line 35) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 65)))) - (export (symbol "jerboa_inotify_close") (file "jerboa-native-rs/src/inotify_native.rs") (line 208) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 71)))) + (export (symbol "jerboa_inotify_close") (file "jerboa-native-rs/src/inotify_native.rs") (line 204) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 71)))) (export (symbol "jerboa_inotify_init") (file "jerboa-native-rs/src/inotify_native.rs") (line 17) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 63)))) - (export (symbol "jerboa_inotify_read") (file "jerboa-native-rs/src/inotify_native.rs") (line 106) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 69)))) - (export (symbol "jerboa_inotify_rm_watch") (file "jerboa-native-rs/src/inotify_native.rs") (line 85) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 67)))) + (export (symbol "jerboa_inotify_read") (file "jerboa-native-rs/src/inotify_native.rs") (line 103) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 69)))) + (export (symbol "jerboa_inotify_rm_watch") (file "jerboa-native-rs/src/inotify_native.rs") (line 82) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 67)))) (export (symbol "jerboa_integrity_hash_file") (file "jerboa-native-rs/src/integrity.rs") (line 322) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 63)))) (export (symbol "jerboa_integrity_hash_region") (file "jerboa-native-rs/src/integrity.rs") (line 224) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 60)))) (export (symbol "jerboa_integrity_hash_self") (file "jerboa-native-rs/src/integrity.rs") (line 68) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 53)))) --- 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`, secure filesystem capability boundary in `secure_fs.rs`, TLS native ABI in `tls.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, wasmi sandbox ABI, SpiderMonkey WASM ABI, DuckDB native ABI, X.509 native ABI, and SQLite native ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 34. | 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`, secure filesystem capability boundary in `secure_fs.rs`, TLS native ABI in `tls.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, wasmi sandbox ABI, SpiderMonkey WASM ABI, DuckDB native ABI, X.509 native ABI, and SQLite native ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 22. | 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/inotify_native.rs +++ b/jerboa-native-rs/src/inotify_native.rs @@ -5,9 +5,9 @@ const MAX_C_ABI_SLICE_LEN: usize = isize::MAX as usize; fn write_event_count(event_count: *mut u8, count: i32) { let bytes = count.to_ne_bytes(); - // SAFETY: jerboa_inotify_read validates that event_count is non-null before - // calling this helper. The Scheme binding passes a 4-byte bytevector, and a - // byte copy avoids imposing i32 alignment on that bytevector. + // jerboa_inotify_read validates event_count is non-null before calling. + // The Scheme binding passes a 4-byte bytevector. + // SAFETY: byte copy avoids imposing i32 alignment on that bytevector. unsafe { std::ptr::copy_nonoverlapping(bytes.as_ptr(), event_count, bytes.len()); } @@ -54,10 +54,8 @@ pub extern "C" fn jerboa_inotify_add_watch( return -1; } }; - // SAFETY: path is non-null, path_len is nonzero and bounded by - // isize::MAX, and the caller contract passes exactly path_len bytes - // from a live Scheme bytevector for the duration of this nonblocking - // syscall wrapper. + // path is non-null, path_len is nonzero and bounded by isize::MAX. + // SAFETY: the Scheme bytevector is live for this synchronous wrapper. let path_bytes = unsafe { std::slice::from_raw_parts(path, path_len) }; if path_bytes.contains(&0) { set_last_error("inotify path contains interior NUL".to_string()); @@ -67,9 +65,8 @@ pub extern "C" fn jerboa_inotify_add_watch( let mut path_buf = Vec::with_capacity(path_capacity); path_buf.extend_from_slice(path_bytes); path_buf.push(0); - // SAFETY: path_buf is explicitly NUL-terminated above and has no - // interior NUL bytes, so libc sees the same path bytes supplied by - // Scheme. inotify_add_watch does not retain the pointer after return. + // path_buf is explicitly NUL-terminated above and has no interior NUL. + // SAFETY: inotify_add_watch does not retain the path pointer. let wd = unsafe { libc::inotify_add_watch(fd, path_buf.as_ptr() as *const _, mask) }; if wd < 0 { set_last_error(format!( @@ -139,9 +136,8 @@ pub extern "C" fn jerboa_inotify_read( } // Parse inotify_event structs and write to output - // SAFETY: output is non-null and output_max is bounded by isize::MAX. - // The caller provides a writable byte buffer of exactly output_max - // bytes; all writes below are checked against output_max. + // output is non-null and output_max is bounded by isize::MAX. + // SAFETY: all writes below are checked against output_max. let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; let mut offset = 0usize; // position in input buf let mut out_pos = 0usize; // position in output --- a/jerboa-native-rs/src/process_ctl.rs +++ b/jerboa-native-rs/src/process_ctl.rs @@ -12,12 +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. + // SAFETY: name is non-null, and len is bounded to the local buffer. 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 + // PR_SET_NAME expects a pointer to a NUL-terminated // 16-byte-or-smaller thread name cast through c_ulong on Linux. + // SAFETY: buf is NUL-terminated and live for the prctl call. let rc = unsafe { libc::prctl(libc::PR_SET_NAME, buf.as_ptr() as libc::c_ulong, 0, 0, 0) }; if rc != 0 { @@ -116,9 +116,9 @@ pub extern "C" fn jerboa_freebsd_is_traced() -> i32 { // 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. + // sysctl receives a fixed MIB, a writable kinfo_proc output buffer + // and length slot, and null new-value pointers. + // SAFETY: this is a read-only query into valid output storage. let rc = unsafe { libc::sysctl( mib.as_mut_ptr(), @@ -218,9 +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. + // setproctitle is called with a constant "%s" format and a + // NUL-terminated byte buffer. + // SAFETY: caller bytes are not interpreted as a format string. unsafe { setproctitle( b"%s\0".as_ptr() as *const libc::c_char, @@ -259,8 +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(). + // output and actual_len are non-null above. + // SAFETY: copy_len is bounded by output_len and path bytes. unsafe { std::ptr::copy_nonoverlapping(path_bytes.as_ptr(), output, copy_len); *actual_len = path_bytes.len(); @@ -284,9 +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. + // sysctl receives a fixed KERN_PROC_PATHNAME MIB, a writable + // pathname buffer and length slot, and null new-value pointers. + // SAFETY: this is a read-only query into valid output storage. let rc = unsafe { libc::sysctl( mib.as_mut_ptr(), --- a/jerboa-native-rs/src/socks5_server.rs +++ b/jerboa-native-rs/src/socks5_server.rs @@ -139,9 +139,8 @@ pub extern "C" fn jerboa_socks5_server_start( set_last_error("bind address length exceeds platform limit".into()); return 0; } - // SAFETY: bind_addr is non-null, bind_addr_len is nonzero and - // bounded by isize::MAX, and the caller must keep the byte buffer - // live for this synchronous conversion. + // bind_addr is non-null, bind_addr_len is nonzero and bounded. + // SAFETY: the caller keeps the buffer live for this conversion. let slice = unsafe { std::slice::from_raw_parts(bind_addr, bind_addr_len) }; match std::str::from_utf8(slice) { Ok(s) => s.to_string(), @@ -161,16 +160,14 @@ pub extern "C" fn jerboa_socks5_server_start( set_last_error("null password with nonzero length".into()); return 0; } - // SAFETY: username is non-null, username_len is nonzero and - // bounded by isize::MAX, and the caller keeps the bytes live for - // this synchronous UTF-8 validation/copy. + // username is non-null, username_len is nonzero and bounded. + // SAFETY: caller keeps bytes live for this UTF-8 validation/copy. let u = unsafe { std::slice::from_raw_parts(username, username_len) }; let p = if password.is_null() { &[] } else { - // SAFETY: password is non-null here, password_len is bounded - // by isize::MAX above, and the bytes are used only for this - // synchronous UTF-8 validation/copy. + // password is non-null here, and password_len is bounded. + // SAFETY: bytes are used only for this UTF-8 validation/copy. unsafe { std::slice::from_raw_parts(password, password_len) } }; match (std::str::from_utf8(u), std::str::from_utf8(p)) { @@ -332,9 +329,8 @@ pub extern "C" fn jerboa_socks5_server_stats(handle: u64, buf: *mut u8, buf_len: let s = format!("active:{} total:{}", active, total); let bytes = s.as_bytes(); let n = bytes.len().min(buf_len); - // SAFETY: buf is non-null, buf_len is bounded by isize::MAX, - // and n is no larger than both the source string and caller - // output buffer. The destination pointer is not retained. + // buf is non-null, and buf_len is bounded by isize::MAX. + // SAFETY: n is capped to source and destination lengths. unsafe { std::ptr::copy_nonoverlapping(bytes.as_ptr(), buf, n); }