security: harden inotify ffi
ober
f3d00feabb7093543aff16c86c82613441690445
--- 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 432) - (unsafe-sites-with-nearby-safety-comment 172) - (unsafe-sites-without-nearby-safety-comment 260) - (unsafe-comment-review-sites 260) + (unsafe-sites 430) + (unsafe-sites-with-nearby-safety-comment 176) + (unsafe-sites-without-nearby-safety-comment 254) + (unsafe-comment-review-sites 254) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -106,8 +106,11 @@ 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 `i64` wrapper for handle-returning exports and documents `jerboa_last_error` -buffer writes. The generated inventory now reports 172 annotated native unsafe -sites and 260 remaining unsafe review sites. +buffer writes. `inotify_native.rs` now documents inotify syscall, path-buffer, +read-buffer, event-count bytevector, and close invariants; it rejects null or +empty paths, oversized lengths, interior NUL path truncation, and null output +pointers before crossing into libc. The generated inventory now reports 176 +annotated native unsafe sites and 254 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,12 @@ not started." 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 `i64` wrapper for handle-returning exports - and documents `jerboa_last_error` buffer writes. The generated report now - shows 172 annotated native unsafe sites and 260 remaining unsafe review sites. + and documents `jerboa_last_error` buffer writes. `inotify_native.rs` now + documents inotify syscall, path-buffer, read-buffer, event-count bytevector, + and close invariants; it rejects null or empty paths, oversized lengths, + interior NUL path truncation, and null output pointers before crossing into + libc. The generated report now shows 176 annotated native unsafe sites and + 254 remaining unsafe review sites. Remaining work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. @@ -1141,7 +1145,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 | 260 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 254 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 @@ -67,11 +67,11 @@ (export (symbol "jerboa_hmac_sha256_verify") (file "jerboa-native-rs/src/crypto.rs") (line 245) (scheme-callers 1 (caller (file "lib/std/crypto/native-rust.ss") (line 120)))) (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 22) (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 158) (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 6) (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 68) (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 49) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 67)))) + (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_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_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`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, embed-crypto ABI, integrity ABI, regex-native ABI, process-control ABI, and pcap ABI now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 260. | 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, and inotify ABI now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 254. | 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 @@ -1,10 +1,23 @@ use crate::panic::{ffi_wrap, set_last_error}; // inotify via libc syscalls (simpler than the inotify crate for FFI) +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. + unsafe { + std::ptr::copy_nonoverlapping(bytes.as_ptr(), event_count, bytes.len()); + } +} #[no_mangle] pub extern "C" fn jerboa_inotify_init() -> i32 { ffi_wrap(|| { + // SAFETY: inotify_init1 has no pointer arguments; the flags are libc + // constants and the returned fd is checked for failure before use. let fd = unsafe { libc::inotify_init1(libc::IN_NONBLOCK | libc::IN_CLOEXEC) }; if fd < 0 { set_last_error(format!( @@ -26,14 +39,37 @@ pub extern "C" fn jerboa_inotify_add_watch( mask: u32, ) -> i32 { ffi_wrap(|| { - if path.is_null() { + if path.is_null() || path_len == 0 { + set_last_error("null or empty inotify path".to_string()); return -1; } + if path_len > MAX_C_ABI_SLICE_LEN { + set_last_error("inotify path length exceeds platform limit".to_string()); + return -1; + } + let path_capacity = match path_len.checked_add(1) { + Some(capacity) => capacity, + None => { + set_last_error("inotify path length overflow".to_string()); + 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. 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()); + return -1; + } // Need null-terminated path for syscall - let mut path_buf = Vec::with_capacity(path_len + 1); + 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. let wd = unsafe { libc::inotify_add_watch(fd, path_buf.as_ptr() as *const _, mask) }; if wd < 0 { set_last_error(format!( @@ -48,6 +84,8 @@ pub extern "C" fn jerboa_inotify_add_watch( #[no_mangle] pub extern "C" fn jerboa_inotify_rm_watch(fd: i32, wd: i32) -> i32 { ffi_wrap(|| { + // SAFETY: inotify_rm_watch takes only integer descriptors; errors from + // invalid descriptors/watch ids are reported by libc and surfaced below. let rc = unsafe { libc::inotify_rm_watch(fd, wd) }; if rc < 0 { set_last_error(format!( @@ -69,35 +107,41 @@ pub extern "C" fn jerboa_inotify_read( fd: i32, output: *mut u8, output_max: usize, - event_count: *mut i32, + event_count: *mut u8, ) -> i32 { ffi_wrap(|| { if output.is_null() || event_count.is_null() { + set_last_error("null inotify output pointer".to_string()); + return -1; + } + if output_max > MAX_C_ABI_SLICE_LEN { + set_last_error("inotify output buffer length exceeds platform limit".to_string()); return -1; } // Read raw events from kernel let mut buf = vec![0u8; 4096]; + // SAFETY: buf is a live Vec<u8> with length buf.len(); libc::read + // writes at most that many bytes and does not retain the pointer. let n = unsafe { libc::read(fd, buf.as_mut_ptr() as *mut _, buf.len()) }; if n < 0 { let err = std::io::Error::last_os_error(); if err.raw_os_error() == Some(libc::EAGAIN) { - unsafe { - *event_count = 0; - } + write_event_count(event_count, 0); return 0; } set_last_error(format!("inotify read: {}", err)); return -1; } if n == 0 { - unsafe { - *event_count = 0; - } + write_event_count(event_count, 0); return 0; } // 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. 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 @@ -116,7 +160,11 @@ pub extern "C" fn jerboa_inotify_read( // Name includes padding; find actual string length let name_start = offset + 16; - let name_end = name_start + name_len_raw as usize; + let name_len = name_len_raw as usize; + let name_end = match name_start.checked_add(name_len) { + Some(end) => end, + None => break, + }; if name_end > n as usize { break; } @@ -130,7 +178,11 @@ pub extern "C" fn jerboa_inotify_read( // Output: wd(4) + mask(4) + name_len(4) + name(actual_name_len) let entry_size = 12 + actual_name_len; - if out_pos + entry_size > output_max { + let entry_end = match out_pos.checked_add(entry_size) { + Some(end) => end, + None => break, + }; + if entry_end > output_max { break; } @@ -142,14 +194,12 @@ pub extern "C" fn jerboa_inotify_read( .copy_from_slice(&name_bytes[..actual_name_len]); } - out_pos += entry_size; + out_pos = entry_end; offset = name_end; count += 1; } - unsafe { - *event_count = count; - } + write_event_count(event_count, count); 0 }) } @@ -157,6 +207,8 @@ pub extern "C" fn jerboa_inotify_read( #[no_mangle] pub extern "C" fn jerboa_inotify_close(fd: i32) -> i32 { ffi_wrap(|| { + // SAFETY: close takes only the integer file descriptor. libc reports + // invalid or already-closed descriptors via the return code. let rc = unsafe { libc::close(fd) }; if rc < 0 { set_last_error(format!("close: {}", std::io::Error::last_os_error())); @@ -164,3 +216,52 @@ pub extern "C" fn jerboa_inotify_close(fd: i32) -> i32 { rc }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ffi_add_watch_rejects_null_path_before_syscall() { + assert_eq!( + jerboa_inotify_add_watch(-1, std::ptr::null(), 3, libc::IN_MODIFY), + -1 + ); + } + + #[test] + fn ffi_add_watch_rejects_empty_path_before_syscall() { + let path = b""; + assert_eq!( + jerboa_inotify_add_watch(-1, path.as_ptr(), path.len(), libc::IN_MODIFY), + -1 + ); + } + + #[test] + fn ffi_add_watch_rejects_interior_nul_before_syscall() { + let path = b"/tmp\0shadow"; + assert_eq!( + jerboa_inotify_add_watch(-1, path.as_ptr(), path.len(), libc::IN_MODIFY), + -1 + ); + } + + #[test] + fn ffi_read_rejects_null_output_before_syscall() { + let mut count = [0u8; 4]; + assert_eq!( + jerboa_inotify_read(-1, std::ptr::null_mut(), 16, count.as_mut_ptr()), + -1 + ); + } + + #[test] + fn ffi_read_rejects_null_count_before_syscall() { + let mut output = [0u8; 16]; + assert_eq!( + jerboa_inotify_read(-1, output.as_mut_ptr(), output.len(), std::ptr::null_mut()), + -1 + ); + } +}