security: harden compression ffi buffers
ober
449b638f3d43377defc66f94d8502b22823ca43f
--- 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 438) - (unsafe-sites-with-nearby-safety-comment 111) - (unsafe-sites-without-nearby-safety-comment 327) - (unsafe-comment-review-sites 327) + (unsafe-sites-with-nearby-safety-comment 123) + (unsafe-sites-without-nearby-safety-comment 315) + (unsafe-comment-review-sites 315) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -85,8 +85,10 @@ seal output length addition for overflow. `secure_mem.rs` now documents the region lifecycle. `ed25519.rs` and `x25519.rs` now document the key buffer, signature output, shared-secret output, and HKDF slice/output invariants; the HKDF wrapper now also rejects null optional buffers with nonzero lengths. The -generated inventory now reports 111 annotated native unsafe sites and 327 -remaining unsafe review sites. +compression FFI wrappers now document input/output/result-slot invariants and +reject oversized decompression caps before `u64` overflow. The generated +inventory now reports 123 annotated native unsafe sites and 315 remaining +unsafe review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -628,9 +628,11 @@ not started." overflow checks. `secure_mem.rs` now documents the mmap/mlock/guard-page, wipe, free, and random-fill unsafe invariants. `ed25519.rs` and `x25519.rs` now document the key/signature/shared-secret/HKDF FFI buffer invariants, and - HKDF rejects null optional buffers with nonzero lengths. The generated report - now shows 111 annotated native unsafe sites and 327 remaining unsafe review - sites. Remaining work: continue unsafe invariant comments across the rest of + HKDF rejects null optional buffers with nonzero lengths. `compress.rs` now + documents compression/decompression FFI buffer invariants and rejects + oversized decompression caps before `u64` overflow. The generated report now + shows 123 annotated native unsafe sites and 315 remaining unsafe review sites. + Remaining work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. ### K3-P1-02 — TOCTOU-safe filesystem capability checks @@ -1119,7 +1121,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 | 327 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 315 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 @@ -61,12 +61,12 @@ (export (symbol "jerboa_eventfd_create") (file "jerboa-native-rs/src/epoll.rs") (line 94) (scheme-callers 1 (caller (file "lib/std/os/epoll-native.ss") (line 293)))) (export (symbol "jerboa_eventfd_drain") (file "jerboa-native-rs/src/epoll.rs") (line 120) (scheme-callers 1 (caller (file "lib/std/os/epoll-native.ss") (line 299)))) (export (symbol "jerboa_eventfd_signal") (file "jerboa-native-rs/src/epoll.rs") (line 106) (scheme-callers 1 (caller (file "lib/std/os/epoll-native.ss") (line 296)))) - (export (symbol "jerboa_gunzip") (file "jerboa-native-rs/src/compress.rs") (line 137) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 39)))) - (export (symbol "jerboa_gzip") (file "jerboa-native-rs/src/compress.rs") (line 97) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 36)))) + (export (symbol "jerboa_gunzip") (file "jerboa-native-rs/src/compress.rs") (line 162) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 39)))) + (export (symbol "jerboa_gzip") (file "jerboa-native-rs/src/compress.rs") (line 116) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 36)))) (export (symbol "jerboa_hmac_sha256") (file "jerboa-native-rs/src/crypto.rs") (line 206) (scheme-callers 2 (caller (file "lib/std/crypto/native-rust.ss") (line 109)) (caller (file "lib/std/crypto/native-rust.ss") (line 120)))) (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 25) (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 49) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 33)))) + (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)))) --- 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`, and Ed25519/X25519 key-agreement buffers now have nearby `SAFETY:` comments, reducing the generated unannotated unsafe-site count to 327. | 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, and compression buffers now have nearby `SAFETY:` comments, reducing the generated unannotated unsafe-site count to 315. | 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/compress.rs +++ b/jerboa-native-rs/src/compress.rs @@ -20,6 +20,8 @@ pub extern "C" fn jerboa_deflate( let data = if input_len == 0 { &[] as &[u8] } else { + // SAFETY: nonzero input_len requires a non-null input pointer + // checked above; callers provide a readable input buffer. unsafe { std::slice::from_raw_parts(input, input_len) } }; let mut encoder = DeflateEncoder::new(data, Compression::default()); @@ -30,8 +32,12 @@ pub extern "C" fn jerboa_deflate( set_last_error("output buffer too small".to_string()); return -1; } + // SAFETY: output is non-null above and the C ABI contract + // requires a writable output_max-byte buffer. let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; out[..buf.len()].copy_from_slice(&buf); + // SAFETY: output_len is non-null above and points to a writable + // usize result slot. unsafe { *output_len = buf.len(); } @@ -60,13 +66,22 @@ pub extern "C" fn jerboa_inflate( let data = if input_len == 0 { &[] as &[u8] } else { + // SAFETY: nonzero input_len requires a non-null input pointer + // checked above; callers provide a readable deflate stream. unsafe { std::slice::from_raw_parts(input, input_len) } }; let decoder = DeflateDecoder::new(data); // Read with size cap to prevent decompression bombs let mut buf = Vec::new(); let cap = output_max; - match decoder.take(cap as u64 + 1).read_to_end(&mut buf) { + let read_limit = match (cap as u64).checked_add(1) { + Some(n) => n, + None => { + set_last_error("output limit too large".to_string()); + return -1; + } + }; + match decoder.take(read_limit).read_to_end(&mut buf) { Ok(_) => { if buf.len() > cap { set_last_error(format!( @@ -76,8 +91,12 @@ pub extern "C" fn jerboa_inflate( )); return -2; // size limit exceeded } + // SAFETY: output is non-null above and the C ABI contract + // requires a writable output_max-byte buffer. let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; out[..buf.len()].copy_from_slice(&buf); + // SAFETY: output_len is non-null above and points to a writable + // usize result slot. unsafe { *output_len = buf.len(); } @@ -108,6 +127,8 @@ pub extern "C" fn jerboa_gzip( let data = if input_len == 0 { &[] as &[u8] } else { + // SAFETY: nonzero input_len requires a non-null input pointer + // checked above; callers provide a readable input buffer. unsafe { std::slice::from_raw_parts(input, input_len) } }; let mut encoder = GzEncoder::new(data, Compression::default()); @@ -118,8 +139,12 @@ pub extern "C" fn jerboa_gzip( set_last_error("output buffer too small".to_string()); return -1; } + // SAFETY: output is non-null above and the C ABI contract + // requires a writable output_max-byte buffer. let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; out[..buf.len()].copy_from_slice(&buf); + // SAFETY: output_len is non-null above and points to a writable + // usize result slot. unsafe { *output_len = buf.len(); } @@ -148,12 +173,21 @@ pub extern "C" fn jerboa_gunzip( let data = if input_len == 0 { &[] as &[u8] } else { + // SAFETY: nonzero input_len requires a non-null input pointer + // checked above; callers provide a readable gzip stream. unsafe { std::slice::from_raw_parts(input, input_len) } }; let decoder = GzDecoder::new(data); let mut buf = Vec::new(); let cap = output_max; - match decoder.take(cap as u64 + 1).read_to_end(&mut buf) { + let read_limit = match (cap as u64).checked_add(1) { + Some(n) => n, + None => { + set_last_error("output limit too large".to_string()); + return -1; + } + }; + match decoder.take(read_limit).read_to_end(&mut buf) { Ok(_) => { if buf.len() > cap { set_last_error(format!( @@ -163,8 +197,12 @@ pub extern "C" fn jerboa_gunzip( )); return -2; } + // SAFETY: output is non-null above and the C ABI contract + // requires a writable output_max-byte buffer. let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; out[..buf.len()].copy_from_slice(&buf); + // SAFETY: output_len is non-null above and points to a writable + // usize result slot. unsafe { *output_len = buf.len(); } @@ -177,3 +215,37 @@ pub extern "C" fn jerboa_gunzip( } }) } + +#[cfg(test)] +mod tests { + use super::{jerboa_gunzip, jerboa_inflate}; + + #[test] + #[cfg(target_pointer_width = "64")] + fn decompression_rejects_overlarge_output_limit() { + let input = [0u8; 1]; + let mut output = [0u8; 1]; + let mut written = 0usize; + + assert_eq!( + jerboa_inflate( + input.as_ptr(), + input.len(), + output.as_mut_ptr(), + usize::MAX, + &mut written, + ), + -1 + ); + assert_eq!( + jerboa_gunzip( + input.as_ptr(), + input.len(), + output.as_mut_ptr(), + usize::MAX, + &mut written, + ), + -1 + ); + } +}