security: harden postgres ffi
ober
ef643e93e801fdc1f2278e7efea1f77be7961a37
--- 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 430) - (unsafe-sites-with-nearby-safety-comment 184) - (unsafe-sites-without-nearby-safety-comment 246) - (unsafe-comment-review-sites 246) + (unsafe-sites 429) + (unsafe-sites-with-nearby-safety-comment 189) + (unsafe-sites-without-nearby-safety-comment 240) + (unsafe-comment-review-sites 240) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -117,8 +117,12 @@ 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. The generated inventory now reports 184 annotated native unsafe sites -and 246 remaining unsafe review sites. +locks. `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. The generated inventory now +reports 189 annotated native unsafe sites and 240 remaining unsafe review +sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -662,8 +662,12 @@ not started." 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. The generated report now shows 184 annotated native unsafe sites and - 246 remaining unsafe review sites. + locks. `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. The generated + report now shows 189 annotated native unsafe sites and 240 remaining unsafe + review sites. Remaining work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. --- a/docs/reviews/2026-07-27-native-export-review.sexp +++ b/docs/reviews/2026-07-27-native-export-review.sexp @@ -89,16 +89,16 @@ (export (symbol "jerboa_pcap_list_interfaces") (file "jerboa-native-rs/src/pcap_capture.rs") (line 371) (scheme-callers 1 (caller (file "lib/std/pcap.ss") (line 62)))) (export (symbol "jerboa_pcap_next") (file "jerboa-native-rs/src/pcap_capture.rs") (line 269) (scheme-callers 1 (caller (file "lib/std/pcap.ss") (line 53)))) (export (symbol "jerboa_pcap_open") (file "jerboa-native-rs/src/pcap_capture.rs") (line 200) (scheme-callers 1 (caller (file "lib/std/pcap.ss") (line 49)))) - (export (symbol "jerboa_pg_column_name") (file "jerboa-native-rs/src/postgres_native.rs") (line 310) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 41)))) - (export (symbol "jerboa_pg_connect") (file "jerboa-native-rs/src/postgres_native.rs") (line 83) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 24)))) - (export (symbol "jerboa_pg_disconnect") (file "jerboa-native-rs/src/postgres_native.rs") (line 132) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 26)))) - (export (symbol "jerboa_pg_exec") (file "jerboa-native-rs/src/postgres_native.rs") (line 145) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 28)))) - (export (symbol "jerboa_pg_free_result") (file "jerboa-native-rs/src/postgres_native.rs") (line 346) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 44)))) - (export (symbol "jerboa_pg_get_value") (file "jerboa-native-rs/src/postgres_native.rs") (line 237) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 36)))) - (export (symbol "jerboa_pg_is_null") (file "jerboa-native-rs/src/postgres_native.rs") (line 286) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 39)))) - (export (symbol "jerboa_pg_ncols") (file "jerboa-native-rs/src/postgres_native.rs") (line 223) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 34)))) - (export (symbol "jerboa_pg_nrows") (file "jerboa-native-rs/src/postgres_native.rs") (line 215) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 32)))) - (export (symbol "jerboa_pg_query") (file "jerboa-native-rs/src/postgres_native.rs") (line 172) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 30)))) + (export (symbol "jerboa_pg_column_name") (file "jerboa-native-rs/src/postgres_native.rs") (line 341) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 41)))) + (export (symbol "jerboa_pg_connect") (file "jerboa-native-rs/src/postgres_native.rs") (line 95) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 24)))) + (export (symbol "jerboa_pg_disconnect") (file "jerboa-native-rs/src/postgres_native.rs") (line 142) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 26)))) + (export (symbol "jerboa_pg_exec") (file "jerboa-native-rs/src/postgres_native.rs") (line 155) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 28)))) + (export (symbol "jerboa_pg_free_result") (file "jerboa-native-rs/src/postgres_native.rs") (line 393) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 44)))) + (export (symbol "jerboa_pg_get_value") (file "jerboa-native-rs/src/postgres_native.rs") (line 250) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 36)))) + (export (symbol "jerboa_pg_is_null") (file "jerboa-native-rs/src/postgres_native.rs") (line 317) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 39)))) + (export (symbol "jerboa_pg_ncols") (file "jerboa-native-rs/src/postgres_native.rs") (line 236) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 34)))) + (export (symbol "jerboa_pg_nrows") (file "jerboa-native-rs/src/postgres_native.rs") (line 228) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 32)))) + (export (symbol "jerboa_pg_query") (file "jerboa-native-rs/src/postgres_native.rs") (line 182) (scheme-callers 1 (caller (file "lib/std/db/postgresql-native.ss") (line 30)))) (export (symbol "jerboa_random_bytes") (file "jerboa-native-rs/src/crypto.rs") (line 285) (scheme-callers 1 (caller (file "lib/std/crypto/native-rust.ss") (line 98)))) (export (symbol "jerboa_regex_captures") (file "jerboa-native-rs/src/regex_native.rs") (line 357) (scheme-callers 2 (caller (file "lib/std/regex-native.ss") (line 40)) (caller (file "lib/std/regex.ss") (line 106)))) (export (symbol "jerboa_regex_compile") (file "jerboa-native-rs/src/regex_native.rs") (line 38) (scheme-callers 3 (caller (file "lib/std/regex-native.ss") (line 27)) (caller (file "lib/std/regex.ss") (line 81)) (caller (file "lib/std/regex.ss") (line 85)))) --- 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, pcap ABI, inotify ABI, epoll/eventfd ABI, antidebug ABI, and SOCKS5 server ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 246. | 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, inotify ABI, epoll/eventfd ABI, antidebug ABI, SOCKS5 server ABI, and PostgreSQL native ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 240. | 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/postgres_native.rs +++ b/jerboa-native-rs/src/postgres_native.rs @@ -6,6 +6,8 @@ use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::LazyLock; use std::sync::{Mutex, MutexGuard}; +const MAX_C_ABI_SLICE_LEN: usize = isize::MAX as usize; + // Handle stores static PG_STORE: LazyLock<Mutex<HashMap<u64, Client>>> = LazyLock::new(|| Mutex::new(HashMap::new())); @@ -26,10 +28,20 @@ fn result_store() -> MutexGuard<'static, HashMap<u64, Vec<Row>>> { } fn ffi_utf8(ptr: *const u8, len: usize, label: &str) -> Result<String, ()> { - if ptr.is_null() { + if len > MAX_C_ABI_SLICE_LEN { + set_last_error(format!("{label} too large")); + return Err(()); + } + if ptr.is_null() && len != 0 { set_last_error(format!("null {label}")); return Err(()); } + if len == 0 { + return Ok(String::new()); + } + // SAFETY: len is bounded to `isize::MAX`, and null pointers with nonzero + // length were rejected above. The returned slice is copied into an owned + // String before this FFI wrapper returns. let bytes = unsafe { std::slice::from_raw_parts(ptr, len) }; std::str::from_utf8(bytes).map(str::to_owned).map_err(|_| { set_last_error(format!("invalid UTF-8 {label}")); @@ -86,19 +98,15 @@ pub extern "C" fn jerboa_pg_connect( handle: *mut u64, ) -> i32 { ffi_wrap(|| { - if connstr.is_null() || handle.is_null() { + if handle.is_null() { + set_last_error("null PostgreSQL handle output".into()); return -1; } - let s = match std::str::from_utf8(unsafe { - std::slice::from_raw_parts(connstr, connstr_len) - }) { + let s = match ffi_utf8(connstr, connstr_len, "PostgreSQL connection string") { Ok(value) => value, - Err(_) => { - set_last_error("invalid UTF-8 PostgreSQL connection string".into()); - return -1; - } + Err(()) => return -1, }; - let (config, cleartext) = match secure_config(s) { + let (config, cleartext) = match secure_config(&s) { Ok(value) => value, Err(error) => { set_last_error(error); @@ -115,6 +123,8 @@ pub extern "C" fn jerboa_pg_connect( Ok(client) => { let id = next_id(); pg_store().insert(id, client); + // SAFETY: `handle` was checked non-null above and points to + // caller-owned storage for the C ABI `uint64_t` result slot. unsafe { *handle = id; } @@ -177,6 +187,7 @@ pub extern "C" fn jerboa_pg_query( ) -> i32 { ffi_wrap(|| { if result_handle.is_null() { + set_last_error("null PostgreSQL result output".into()); return -1; } let sql_str = match ffi_utf8(sql, sql_len, "PostgreSQL SQL") { @@ -196,6 +207,8 @@ pub extern "C" fn jerboa_pg_query( let id = next_id(); drop(store); result_store().insert(id, rows); + // SAFETY: `result_handle` was checked non-null above and + // points to caller-owned storage for the C ABI result slot. unsafe { *result_handle = id; } @@ -243,7 +256,16 @@ pub extern "C" fn jerboa_pg_get_value( output_len: *mut usize, ) -> i32 { ffi_wrap(|| { - if output.is_null() || output_len.is_null() { + if output_len.is_null() { + set_last_error("null PostgreSQL value length output".into()); + return -1; + } + if output.is_null() && output_max != 0 { + set_last_error("null PostgreSQL value output".into()); + return -1; + } + if output_max > MAX_C_ABI_SLICE_LEN { + set_last_error("PostgreSQL value output buffer too large".into()); return -1; } let store = result_store(); @@ -263,6 +285,8 @@ pub extern "C" fn jerboa_pg_get_value( let val: Option<String> = r.try_get(col as usize).unwrap_or(None); match val { None => { + // SAFETY: `output_len` was checked non-null above and points + // to caller-owned storage for a `usize` length result. unsafe { *output_len = 0; } @@ -271,8 +295,15 @@ pub extern "C" fn jerboa_pg_get_value( Some(s) => { let bytes = s.as_bytes(); let copy_len = bytes.len().min(output_max); - let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; - out[..copy_len].copy_from_slice(&bytes[..copy_len]); + if copy_len != 0 { + // SAFETY: null output with nonzero capacity was rejected, + // and `output_max` is bounded to `isize::MAX`; the slice + // is used only for this bounded copy. + let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; + out[..copy_len].copy_from_slice(&bytes[..copy_len]); + } + // SAFETY: `output_len` was checked non-null above and points + // to caller-owned storage for a `usize` length result. unsafe { *output_len = bytes.len(); } @@ -315,7 +346,16 @@ pub extern "C" fn jerboa_pg_column_name( output_len: *mut usize, ) -> i32 { ffi_wrap(|| { - if output.is_null() || output_len.is_null() { + if output_len.is_null() { + set_last_error("null PostgreSQL column-name length output".into()); + return -1; + } + if output.is_null() && output_max != 0 { + set_last_error("null PostgreSQL column-name output".into()); + return -1; + } + if output_max > MAX_C_ABI_SLICE_LEN { + set_last_error("PostgreSQL column-name output buffer too large".into()); return -1; } let store = result_store(); @@ -333,8 +373,15 @@ pub extern "C" fn jerboa_pg_column_name( let name = columns[col as usize].name(); let bytes = name.as_bytes(); let copy_len = bytes.len().min(output_max); - let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; - out[..copy_len].copy_from_slice(&bytes[..copy_len]); + if copy_len != 0 { + // SAFETY: null output with nonzero capacity was rejected, and + // `output_max` is bounded to `isize::MAX`; the slice is used only + // for this bounded copy. + let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; + out[..copy_len].copy_from_slice(&bytes[..copy_len]); + } + // SAFETY: `output_len` was checked non-null above and points to + // caller-owned storage for a `usize` length result. unsafe { *output_len = bytes.len(); } @@ -371,6 +418,7 @@ mod security_tests { #[test] fn sql_ffi_rejects_null_and_invalid_utf8_before_handle_lookup() { assert_eq!(jerboa_pg_exec(u64::MAX, std::ptr::null(), 0), -1); + assert_eq!(jerboa_pg_exec(u64::MAX, std::ptr::null(), 1), -1); let invalid = [0xffu8]; assert_eq!( jerboa_pg_exec(u64::MAX, invalid.as_ptr(), invalid.len()), @@ -379,6 +427,28 @@ mod security_tests { } #[test] + fn connect_rejects_invalid_c_abi_outputs() { + assert_eq!( + jerboa_pg_connect(std::ptr::null(), 0, std::ptr::null_mut()), + -1 + ); + let mut handle = 0u64; + assert_eq!( + jerboa_pg_connect(std::ptr::null(), 1, &mut handle as *mut u64), + -1 + ); + } + + #[test] + fn query_rejects_invalid_result_output() { + let sql = b"select 1"; + assert_eq!( + jerboa_pg_query(u64::MAX, sql.as_ptr(), sql.len(), std::ptr::null_mut()), + -1 + ); + } + + #[test] fn result_store_poison_does_not_cross_ffi_boundary() { let _ = std::panic::catch_unwind(|| { let _guard = RESULT_STORE