security: harden aproc ffi buffers
ober
1425fc670576be17fa4a041630edbc40c80eb375
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -29,13 +29,13 @@ Latest summary from `tools/ffi-audit-report.ss`: (load-shared-object 55) (define-ftype 4) (foreign-callable 7) - (pointer-sites 287) + (pointer-sites 290) (width-sensitive-sites 145) - (blocking-candidates 59) + (blocking-candidates 62) (blocking-without-collect-safe 0) - (hazard-site-count 340) + (hazard-site-count 343) (blocking-review-sites 0) - (safety-review-sites 340)) + (safety-review-sites 343)) (native (rust-file-count 29) (c-file-count 5) @@ -71,7 +71,7 @@ caller reconciliation. The Scheme source inventory now ignores generated `.sls` mirrors and string literals in code generators, so the counts reflect source FFI forms instead of -documentation or emitted-code text. All 59 blocking candidate bindings are now +documentation or emitted-code text. All 62 blocking candidate bindings are now either `__collect_safe` or classified as nonblocking by the scanner; the generated report has 0 `blocking-without-collect-safe` sites. @@ -211,7 +211,10 @@ pointer before dereferencing it, ensures fds 0/1/2 are open after `setsid`, runs collect-safe `read`/`write` through scoped foreign `void*` buffers, copies `stat`/`fstat`/`lstat` results out of scoped native storage into bytevectors, and keeps `free-stat` as a compatibility no-op for callers using the exported -stat accessors. +stat accessors. The Scheme `(std os aproc)` wrapper now routes collect-safe +`read`, `write`, and `waitpid` through scoped foreign `void*` buffers/status +slots while preserving the existing high-level API, and checks slice bounds +before subprocess write copies. `secure_fs.rs` now documents descriptor-relative `openat`/`mkdirat`, fd duplication/ownership transfer, stat buffers, directory stream lifecycle, @@ -223,7 +226,7 @@ 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 390 annotated native unsafe sites, 0 remaining unsafe review sites, and 0 Scheme blocking calls missing `__collect_safe`; the clean-tree Scheme FFI -inventory now reports 340 remaining provisional safety-review sites. +inventory now reports 343 remaining provisional safety-review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -751,7 +751,10 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit `/dev/null` after `setsid`, routes collect-safe `read`/`write` through scoped foreign `void*` buffers, validates terminal and stat buffer bounds, and returns `stat`/`fstat`/`lstat` data as bytevectors copied out of scoped - native storage. + native storage. The Scheme `(std os aproc)` wrapper now routes collect-safe + `read`, `write`, and `waitpid` through checked Scheme wrappers over scoped + foreign `void*` buffers/status slots, and validates subprocess write slice + bounds before copying. `wasm_sm.rs` now compiles under `unsafe_op_in_unsafe_fn` for the SpiderMonkey feature, bounds C ABI buffers, validates host-memory offsets, converts poisoned handle-store locks into normal FFI errors, and documents @@ -768,7 +771,7 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit shutdown invariants for blocked TLS I/O. The generated report now shows 390 annotated native unsafe sites, 0 remaining unsafe review sites, and 0 Scheme blocking calls missing `__collect_safe`; the clean-tree Scheme FFI inventory - now reports 59 blocking candidates, 0 blocking-review sites, and 340 + now reports 62 blocking candidates, 0 blocking-review sites, and 343 remaining provisional safety-review sites. Remaining work: finish per-binding Scheme FFI review and targeted scanner rules. --- a/docs/security-reference.md +++ b/docs/security-reference.md @@ -718,7 +718,7 @@ Callers should not treat `allow-degraded?` as successful sandbox installation. ## 7. Parser Hardening -Phases 1-4 are implemented and tested (42 tests in `tests/test-security2-parsers.ss`). Phase 5 (FFI audit) has a reproducible inventory in [ffi-audit.md](ffi-audit.md) via `make ffi-audit-report`; native unsafe review and selected Scheme FFI slices such as Seatbelt, sandbox compatibility, `exec-id`, `regex-native`, the unified regex facade, `compress native-rust`, `crypto native-rust`, `net tls`, `net tls-rustls`, and `std os posix` are remediated, while broad Scheme per-binding remediation remains open. +Phases 1-4 are implemented and tested (42 tests in `tests/test-security2-parsers.ss`). Phase 5 (FFI audit) has a reproducible inventory in [ffi-audit.md](ffi-audit.md) via `make ffi-audit-report`; native unsafe review and selected Scheme FFI slices such as Seatbelt, sandbox compatibility, `exec-id`, `regex-native`, the unified regex facade, `compress native-rust`, `crypto native-rust`, `net tls`, `net tls-rustls`, `std os posix`, and `std os aproc` are remediated, while broad Scheme per-binding remediation remains open. ### Depth limits @@ -1000,7 +1000,7 @@ These are known gaps documented as current limitations, not implementation promi `(std crypto native-rust)` / `(std crypto password)`. The high-level HMAC, AEAD, KDF, and password-hashing compatibility APIs now route through the Rust native crypto boundary. -- **FFI audit (Phase 5 of parser hardening) is in progress.** `make ffi-audit-report` inventories Scheme FFI sites, Rust C ABI exports, pointer/width-sensitive bindings, blocking candidates, and Rust unsafe sites. Native unsafe review and selected Scheme FFI slices, including the regex, compression, crypto, OpenSSL TLS, rustls TLS, and POSIX wrappers, are remediated; the remaining Scheme per-binding null-return, bounds, ownership, and GC-safety review is tracked in [ffi-audit.md](ffi-audit.md). +- **FFI audit (Phase 5 of parser hardening) is in progress.** `make ffi-audit-report` inventories Scheme FFI sites, Rust C ABI exports, pointer/width-sensitive bindings, blocking candidates, and Rust unsafe sites. Native unsafe review and selected Scheme FFI slices, including the regex, compression, crypto, OpenSSL TLS, rustls TLS, POSIX, and async-process wrappers, are remediated; the remaining Scheme per-binding null-return, bounds, ownership, and GC-safety review is tracked in [ffi-audit.md](ffi-audit.md). - **No red team evaluation.** No independent adversarial testing has been performed. - **Secure memory still exposes a raw region escape hatch.** The high-level `secure-bytevector` API is bounds-checked and integrated with --- 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, SQLite native ABI, the Scheme Seatbelt `sandbox_init` error-buffer paths, the retired `(std os sandbox)` compatibility surface, `(std os exec-id)` realpath/hash helpers, and the Scheme `(std regex-native)`, `(std regex)`, `(std compress native-rust)`, `(std crypto native-rust)`, `(std net tls)`, `(std net tls-rustls)`, and `(std os posix)` out-parameter or foreign-buffer wrappers now have nearby `SAFETY:` comments or equivalent checked FFI/scanner invariants. Generated counts are at 0 unannotated native unsafe sites, 0 Scheme blocking bindings missing `__collect_safe`, and 340 remaining provisional Scheme safety-review sites. | Finish per-binding Scheme FFI review and targeted scanner rules for the remaining provisional verdicts. | +| 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, SQLite native ABI, the Scheme Seatbelt `sandbox_init` error-buffer paths, the retired `(std os sandbox)` compatibility surface, `(std os exec-id)` realpath/hash helpers, and the Scheme `(std regex-native)`, `(std regex)`, `(std compress native-rust)`, `(std crypto native-rust)`, `(std net tls)`, `(std net tls-rustls)`, `(std os posix)`, and `(std os aproc)` out-parameter or foreign-buffer wrappers now have nearby `SAFETY:` comments or equivalent checked FFI/scanner invariants. Generated counts are at 0 unannotated native unsafe sites, 0 Scheme blocking bindings missing `__collect_safe`, and 343 remaining provisional Scheme safety-review sites. | Finish per-binding Scheme FFI review and targeted scanner rules for the remaining provisional verdicts. | | 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, Linux Landlock filesystem/TCP-connect setup for requested axes, macOS Seatbelt deny-default path/exec/no-network setup for supported axes, standard worker-eval Capsicum entry on FreeBSD, explicit sandbox-axis refusal, egress proxy env wiring, and platform CI smoke for Linux/macOS/FreeBSD sandbox paths. | Keep Linux/macOS/FreeBSD parity tests current; finish arbitrary-command/proxy-aware Capsicum worker paths. | | Fuzzing | `tests/fuzz/corpus/` has 15 checked-in seed inputs, `tests/fuzz/regression/` has 11 crash/rejection regressions, `make fuzz-smoke` runs the deterministic regression gate first, and GitHub CI runs smoke fuzzing normally plus deep fuzzing on scheduled daily runs. | Keep adding minimized corpus and regression inputs for every parser/security bug found. | --- a/lib/std/os/aproc.ss +++ b/lib/std/os/aproc.ss @@ -105,15 +105,83 @@ ;; ========== FFI: libc ========== ;; __collect_safe releases the TC mutex while the call is parked in the ;; kernel, so other green threads keep running. - (def c-read - (foreign-procedure __collect_safe "read" - (int u8* unsigned-int) int)) - (def c-write - (foreign-procedure __collect_safe "write" - (int u8* unsigned-int) int)) - (def c-waitpid - (foreign-procedure __collect_safe "waitpid" - (int u8* int) int)) + (def c-read-raw + (foreign-procedure __collect_safe "read" (int void* unsigned-int) int)) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- read receives a scoped foreign buffer, not a movable Scheme bytevector; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- foreign-allocated read buffer is declared void*; jerboa-security: suppress ffi-pointer-return-without-null-guard -- read returns a byte count, not a pointer; jerboa-security: suppress ffi-integer-width-ambiguous -- count is checked against the target bytevector before the FFI call + (def c-write-raw + (foreign-procedure __collect_safe "write" (int void* unsigned-int) int)) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- write receives a scoped foreign buffer, not a movable Scheme bytevector; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- foreign-allocated write buffer is declared void*; jerboa-security: suppress ffi-pointer-return-without-null-guard -- write returns a byte count, not a pointer; jerboa-security: suppress ffi-integer-width-ambiguous -- count is checked against the source bytevector before the FFI call + (def c-waitpid-raw + (foreign-procedure __collect_safe "waitpid" (int void* int) int)) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- waitpid receives a scoped foreign status slot, not a movable Scheme bytevector; jerboa-security: suppress ffi-pointer-return-without-null-guard -- waitpid returns int and fills caller-owned status storage; jerboa-security: suppress ffi-integer-width-ambiguous -- pid/options/status are POSIX int fields, not size or pointer-width counts + + (def (non-null-ptr? ptr) + (and ptr (not (= ptr 0)))) + + (def (call-with-aproc-foreign-buffer who size k) + (unless (and (integer? size) (exact? size) (>= size 0)) + (error who "invalid foreign buffer size" size)) + (let ([ptr #f]) + (dynamic-wind + (lambda () + (set! ptr (foreign-alloc (max 1 size))) ; jerboa-security: suppress foreign-alloc-no-free -- ptr is released by the dynamic-wind cleanup after the blocking syscall returns or raises + (unless (non-null-ptr? ptr) + (error who "foreign buffer allocation failed" size))) + (lambda () + (k ptr)) + (lambda () + (when (non-null-ptr? ptr) + (foreign-free ptr) + (set! ptr #f)))))) + + (def (copy-foreign-to-bytevector! who ptr out len) + (unless (and (bytevector? out) (integer? len) (exact? len) (>= len 0) + (<= len (bytevector-length out))) + (error who "foreign output length exceeds bytevector capacity" len)) + (let loop ([i 0]) + (when (< i len) + (bytevector-u8-set! out i (foreign-ref 'unsigned-8 ptr i)) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- i is bounded by len after len is checked against the target bytevector capacity + (loop (+ i 1))))) + + (def (copy-bytevector-to-foreign! who bv ptr len) + (unless (and (bytevector? bv) (integer? len) (exact? len) (>= len 0) + (<= len (bytevector-length bv))) + (error who "foreign input length exceeds bytevector capacity" len)) + (let loop ([i 0]) + (when (< i len) + (foreign-set! 'unsigned-8 ptr i (bytevector-u8-ref bv i)) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- i is bounded by len after len is checked against the source bytevector capacity + (loop (+ i 1))))) + + (def (c-read fd bv count) + (unless (and (bytevector? bv) (integer? count) (exact? count) (>= count 0) + (<= count (bytevector-length bv))) + (error 'c-read "read count exceeds bytevector capacity" count)) + (if (= count 0) + 0 + (call-with-aproc-foreign-buffer 'c-read count + (lambda (ptr) + (let ([n (c-read-raw fd ptr count)]) + (when (> n 0) + (copy-foreign-to-bytevector! 'c-read ptr bv n)) + n))))) + + (def (c-write fd bv count) + (unless (and (bytevector? bv) (integer? count) (exact? count) (>= count 0) + (<= count (bytevector-length bv))) + (error 'c-write "write count exceeds bytevector capacity" count)) + (if (= count 0) + 0 + (call-with-aproc-foreign-buffer 'c-write count + (lambda (ptr) + (copy-bytevector-to-foreign! 'c-write bv ptr count) + (c-write-raw fd ptr count))))) + + (def (c-waitpid pid status-bv options) + (unless (and (bytevector? status-bv) (>= (bytevector-length status-bv) 4)) + (error 'c-waitpid "expected four-byte status bytevector" status-bv)) + (call-with-aproc-foreign-buffer 'c-waitpid 4 + (lambda (ptr) + (let ([rc (c-waitpid-raw pid ptr options)]) + (when (>= rc 0) + (copy-foreign-to-bytevector! 'c-waitpid ptr status-bv 4)) + rc)))) (def c-system-safe (foreign-procedure __collect_safe "system" (u8*) int)) @@ -144,24 +212,21 @@ (def c-jerboa-aproc-spawn-pty (and _native-loaded (try (foreign-procedure "jerboa_aproc_spawn_pty" - (u8* size_t size_t - u8* size_t size_t - u8* size_t - int int u8*) - int) + (u8* size_t size_t u8* size_t size_t u8* size_t int int u8*) + int) ; jerboa-security: suppress ffi-integer-width-ambiguous -- PTY spawn status/fd modes are fixed int ABI fields; buffer lengths cross as size_t and result storage is a fixed 5-int bytevector (catch (e) #f)))) (def c-jerboa-aproc-set-nonblock (and _native-loaded - (try (foreign-procedure "jerboa_aproc_set_nonblock" (int int) int) + (try (foreign-procedure "jerboa_aproc_set_nonblock" (int int) int) ; jerboa-security: suppress ffi-integer-width-ambiguous -- fd and boolean mode are fixed int ABI fields, not size or pointer-width counts (catch (e) #f)))) (def c-jerboa-aproc-killpg (and _native-loaded - (try (foreign-procedure "jerboa_aproc_killpg" (int int) int) + (try (foreign-procedure "jerboa_aproc_killpg" (int int) int) ; jerboa-security: suppress ffi-integer-width-ambiguous -- process group id and signal are fixed POSIX int ABI fields (catch (e) #f)))) (def c-jerboa-aproc-wait4 (and _native-loaded (try (foreign-procedure __collect_safe "jerboa_aproc_wait4" - (int int u8* u8*) int) + (int int u8* u8*) int) ; jerboa-security: suppress ffi-integer-width-ambiguous -- pid/options are fixed POSIX int ABI fields and status/rusage outputs are fixed-size bytevectors (catch (e) #f)))) (def c-jerboa-last-error (and _native-loaded @@ -247,7 +312,7 @@ [(null? rest) (values buf total (length strs))] [else (let* ([b (car rest)] [n (bytevector-length b)]) - (bytevector-copy! b 0 buf off n) + (bytevector-copy! b 0 buf off n) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- off advances through the precomputed total buffer, and each copied segment contributes n plus one NUL byte (bytevector-u8-set! buf (+ off n) 0) (loop (+ off n 1) (cdr rest)))])))) @@ -681,7 +746,7 @@ ((= n count) buf) (else (let ((r (make-bytevector n))) - (bytevector-copy! buf 0 r 0 n) + (bytevector-copy! buf 0 r 0 n) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- n is positive and no larger than count, and r is allocated to exactly n bytes r))))])) (def (aproc-read-stdout h . maybe-count) @@ -994,7 +1059,7 @@ (lp (+ i 1))] [else (let* ([slice (make-bytevector nbytes)]) - (bytevector-copy! bv i slice 0 nbytes) + (bytevector-copy! bv i slice 0 nbytes) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- preceding guard rejects zero sizes and i+nbytes beyond the source bytevector length (let ([s (try (utf8->string slice) (catch (e) #f))]) (cond @@ -1090,10 +1155,14 @@ [else n]))))) (def (c-write-slice fd bv off len) - ;; c-write takes a u8* base + length. Make a small copy for the slice - ;; (Chez foreign-procedure with u8* expects the buffer base). + (unless (and (bytevector? bv) + (integer? off) (exact? off) (>= off 0) + (integer? len) (exact? len) (>= len 0) + (<= (+ off len) (bytevector-length bv))) + (error 'c-write-slice "slice exceeds bytevector bounds" off len)) + ;; c-write copies to a scoped foreign buffer; this helper first narrows the Scheme slice. (let ([slice (make-bytevector len)]) - (bytevector-copy! bv off slice 0 len) + (bytevector-copy! bv off slice 0 len) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- off and len are checked against the source bytevector before the copy (c-write fd slice len))) (def (build-shell-from-argv argv env env-pure dir)