security: harden duckdb ffi
ober
69150f47bd3c68a2065f951260d90f0d6eacf809
--- 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 427) - (unsafe-sites-with-nearby-safety-comment 198) - (unsafe-sites-without-nearby-safety-comment 229) - (unsafe-comment-review-sites 229) + (unsafe-sites 407) + (unsafe-sites-with-nearby-safety-comment 203) + (unsafe-sites-without-nearby-safety-comment 204) + (unsafe-comment-review-sites 204) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -127,9 +127,13 @@ has Linux-only regression tests for syscall-free input rejection. `wasm.rs` now bounds module/function/path/log buffers, converts poisoned handle-store locks into normal FFI errors, rejects null argument/result buffers with nonzero lengths, checks guest-memory offsets with overflow-safe helpers, -and has Rust regressions for invalid C ABI inputs. The generated inventory now -reports 198 annotated native unsafe sites and 229 remaining unsafe review -sites. +and has Rust regressions for invalid C ABI inputs. `duckdb_native.rs` now +funnels path, SQL, text, and blob inputs through checked C ABI slice helpers, +converts poisoned database/statement/result store locks into normal FFI errors, +rejects null result and output slots, checks result indexes before converting +signed values to `usize`, and centralizes output-buffer writes with bounded +copy invariants. The generated inventory now reports 203 annotated native +unsafe sites and 204 remaining unsafe review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -673,8 +673,11 @@ not started." converts poisoned handle-store locks into normal FFI errors, rejects null argument/result buffers with nonzero lengths, checks guest-memory offsets with overflow-safe helpers, and has Rust regressions for invalid C ABI - inputs. The generated report now shows 198 annotated native unsafe sites and - 229 remaining unsafe review sites. + inputs. `duckdb_native.rs` now validates path, SQL, text, blob, result-slot, + result-index, and output-buffer inputs through shared helpers and converts + poisoned handle-store locks into normal FFI errors. The generated report now + shows 203 annotated native unsafe sites and 204 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 @@ -29,31 +29,31 @@ (export (symbol "jerboa_chacha20_open") (file "jerboa-native-rs/src/crypto.rs") (line 606) (scheme-callers 1 (caller (file "lib/std/crypto/native-rust.ss") (line 204)))) (export (symbol "jerboa_chacha20_seal") (file "jerboa-native-rs/src/crypto.rs") (line 519) (scheme-callers 1 (caller (file "lib/std/crypto/native-rust.ss") (line 182)))) (export (symbol "jerboa_deflate") (file "jerboa-native-rs/src/compress.rs") (line 9) (scheme-callers 1 (caller (file "lib/std/compress/native-rust.ss") (line 30)))) - (export (symbol "jerboa_duckdb_bind_blob") (file "jerboa-native-rs/src/duckdb_native.rs") (line 334) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 62)))) - (export (symbol "jerboa_duckdb_bind_bool") (file "jerboa-native-rs/src/duckdb_native.rs") (line 386) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 66)))) - (export (symbol "jerboa_duckdb_bind_double") (file "jerboa-native-rs/src/duckdb_native.rs") (line 272) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 58)))) - (export (symbol "jerboa_duckdb_bind_int") (file "jerboa-native-rs/src/duckdb_native.rs") (line 251) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 56)))) - (export (symbol "jerboa_duckdb_bind_null") (file "jerboa-native-rs/src/duckdb_native.rs") (line 365) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 64)))) - (export (symbol "jerboa_duckdb_bind_text") (file "jerboa-native-rs/src/duckdb_native.rs") (line 293) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 60)))) - (export (symbol "jerboa_duckdb_close") (file "jerboa-native-rs/src/duckdb_native.rs") (line 125) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 45)))) - (export (symbol "jerboa_duckdb_column_name") (file "jerboa-native-rs/src/duckdb_native.rs") (line 673) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 76)))) - (export (symbol "jerboa_duckdb_column_type") (file "jerboa-native-rs/src/duckdb_native.rs") (line 710) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 79)))) - (export (symbol "jerboa_duckdb_exec") (file "jerboa-native-rs/src/duckdb_native.rs") (line 154) (scheme-callers 2 (caller (file "lib/std/db/duckdb-native.ss") (line 47)) (caller (file "lib/std/db/duckdb-native.ss") (line 69)))) - (export (symbol "jerboa_duckdb_execute") (file "jerboa-native-rs/src/duckdb_native.rs") (line 433) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 69)))) - (export (symbol "jerboa_duckdb_finalize") (file "jerboa-native-rs/src/duckdb_native.rs") (line 968) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 51)))) - (export (symbol "jerboa_duckdb_free_result") (file "jerboa-native-rs/src/duckdb_native.rs") (line 955) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 96)))) - (export (symbol "jerboa_duckdb_ncols") (file "jerboa-native-rs/src/duckdb_native.rs") (line 664) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 74)))) - (export (symbol "jerboa_duckdb_nrows") (file "jerboa-native-rs/src/duckdb_native.rs") (line 655) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 72)))) - (export (symbol "jerboa_duckdb_open") (file "jerboa-native-rs/src/duckdb_native.rs") (line 85) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 43)))) - (export (symbol "jerboa_duckdb_prepare") (file "jerboa-native-rs/src/duckdb_native.rs") (line 190) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 49)))) - (export (symbol "jerboa_duckdb_reset") (file "jerboa-native-rs/src/duckdb_native.rs") (line 980) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 53)))) - (export (symbol "jerboa_duckdb_value_blob") (file "jerboa-native-rs/src/duckdb_native.rs") (line 903) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 92)))) - (export (symbol "jerboa_duckdb_value_bool") (file "jerboa-native-rs/src/duckdb_native.rs") (line 790) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 87)))) - (export (symbol "jerboa_duckdb_value_double") (file "jerboa-native-rs/src/duckdb_native.rs") (line 770) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 85)))) - (export (symbol "jerboa_duckdb_value_int") (file "jerboa-native-rs/src/duckdb_native.rs") (line 744) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 83)))) - (export (symbol "jerboa_duckdb_value_is_null") (file "jerboa-native-rs/src/duckdb_native.rs") (line 725) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 81)))) - (export (symbol "jerboa_duckdb_value_text") (file "jerboa-native-rs/src/duckdb_native.rs") (line 822) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 89)))) - (export (symbol "jerboa_duckdb_version") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1002) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 98)))) + (export (symbol "jerboa_duckdb_bind_blob") (file "jerboa-native-rs/src/duckdb_native.rs") (line 444) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 62)))) + (export (symbol "jerboa_duckdb_bind_bool") (file "jerboa-native-rs/src/duckdb_native.rs") (line 501) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 66)))) + (export (symbol "jerboa_duckdb_bind_double") (file "jerboa-native-rs/src/duckdb_native.rs") (line 387) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 58)))) + (export (symbol "jerboa_duckdb_bind_int") (file "jerboa-native-rs/src/duckdb_native.rs") (line 363) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 56)))) + (export (symbol "jerboa_duckdb_bind_null") (file "jerboa-native-rs/src/duckdb_native.rs") (line 477) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 64)))) + (export (symbol "jerboa_duckdb_bind_text") (file "jerboa-native-rs/src/duckdb_native.rs") (line 411) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 60)))) + (export (symbol "jerboa_duckdb_close") (file "jerboa-native-rs/src/duckdb_native.rs") (line 228) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 45)))) + (export (symbol "jerboa_duckdb_column_name") (file "jerboa-native-rs/src/duckdb_native.rs") (line 855) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 76)))) + (export (symbol "jerboa_duckdb_column_type") (file "jerboa-native-rs/src/duckdb_native.rs") (line 897) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 79)))) + (export (symbol "jerboa_duckdb_exec") (file "jerboa-native-rs/src/duckdb_native.rs") (line 264) (scheme-callers 2 (caller (file "lib/std/db/duckdb-native.ss") (line 47)) (caller (file "lib/std/db/duckdb-native.ss") (line 69)))) + (export (symbol "jerboa_duckdb_execute") (file "jerboa-native-rs/src/duckdb_native.rs") (line 598) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 69)))) + (export (symbol "jerboa_duckdb_finalize") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1124) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 51)))) + (export (symbol "jerboa_duckdb_free_result") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1107) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 96)))) + (export (symbol "jerboa_duckdb_ncols") (file "jerboa-native-rs/src/duckdb_native.rs") (line 843) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 74)))) + (export (symbol "jerboa_duckdb_nrows") (file "jerboa-native-rs/src/duckdb_native.rs") (line 831) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 72)))) + (export (symbol "jerboa_duckdb_open") (file "jerboa-native-rs/src/duckdb_native.rs") (line 185) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 43)))) + (export (symbol "jerboa_duckdb_prepare") (file "jerboa-native-rs/src/duckdb_native.rs") (line 296) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 49)))) + (export (symbol "jerboa_duckdb_reset") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1142) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 53)))) + (export (symbol "jerboa_duckdb_value_blob") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1069) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 92)))) + (export (symbol "jerboa_duckdb_value_bool") (file "jerboa-native-rs/src/duckdb_native.rs") (line 989) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 87)))) + (export (symbol "jerboa_duckdb_value_double") (file "jerboa-native-rs/src/duckdb_native.rs") (line 967) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 85)))) + (export (symbol "jerboa_duckdb_value_int") (file "jerboa-native-rs/src/duckdb_native.rs") (line 939) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 83)))) + (export (symbol "jerboa_duckdb_value_is_null") (file "jerboa-native-rs/src/duckdb_native.rs") (line 918) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 81)))) + (export (symbol "jerboa_duckdb_value_text") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1023) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 89)))) + (export (symbol "jerboa_duckdb_version") (file "jerboa-native-rs/src/duckdb_native.rs") (line 1167) (scheme-callers 1 (caller (file "lib/std/db/duckdb-native.ss") (line 98)))) (export (symbol "jerboa_epoll_close") (file "jerboa-native-rs/src/epoll.rs") (line 107) (scheme-callers 1 (caller (file "lib/std/os/epoll-native.ss") (line 59)))) (export (symbol "jerboa_epoll_create") (file "jerboa-native-rs/src/epoll.rs") (line 7) (scheme-callers 2 (caller (file "lib/std/os/epoll-native.ss") (line 30)) (caller (file "lib/std/os/epoll-native.ss") (line 50)))) (export (symbol "jerboa_epoll_ctl") (file "jerboa-native-rs/src/epoll.rs") (line 25) (scheme-callers 1 (caller (file "lib/std/os/epoll-native.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, pcap ABI, inotify ABI, epoll/eventfd ABI, antidebug ABI, SOCKS5 server ABI, PostgreSQL native ABI, Landlock native ABI, and wasmi sandbox ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 229. | 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, PostgreSQL native ABI, Landlock native ABI, wasmi sandbox ABI, and DuckDB native ABI now have nearby `SAFETY:` comments or equivalent checked FFI invariants, with the generated unannotated unsafe-site count at 204. | 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/duckdb_native.rs +++ b/jerboa-native-rs/src/duckdb_native.rs @@ -3,7 +3,7 @@ use duckdb::{types::Value, Connection}; use std::collections::HashMap; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::LazyLock; -use std::sync::Mutex; +use std::sync::{Mutex, MutexGuard}; // ============================================================ // Handle stores @@ -24,6 +24,8 @@ static RESULT_STORE: LazyLock<Mutex<HashMap<u64, ResultSet>>> = static NEXT_ID: AtomicU64 = AtomicU64::new(1); +const MAX_C_ABI_SLICE_LEN: usize = isize::MAX as usize; + struct PendingStmt { db_handle: u64, sql: String, @@ -77,6 +79,104 @@ fn next_id() -> u64 { NEXT_ID.fetch_add(1, Ordering::SeqCst) } +fn lock_db_store() -> Option<MutexGuard<'static, HashMap<u64, Connection>>> { + match DB_STORE.lock() { + Ok(guard) => Some(guard), + Err(_) => { + set_last_error("duckdb database store lock poisoned".into()); + None + } + } +} + +fn lock_stmt_store() -> Option<MutexGuard<'static, HashMap<u64, PendingStmt>>> { + match STMT_STORE.lock() { + Ok(guard) => Some(guard), + Err(_) => { + set_last_error("duckdb statement store lock poisoned".into()); + None + } + } +} + +fn lock_result_store() -> Option<MutexGuard<'static, HashMap<u64, ResultSet>>> { + match RESULT_STORE.lock() { + Ok(guard) => Some(guard), + Err(_) => { + set_last_error("duckdb result store lock poisoned".into()); + None + } + } +} + +fn ffi_bytes<'a>(ptr: *const u8, len: usize, label: &str) -> Result<&'a [u8], ()> { + 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(&[]); + } + // SAFETY: length is bounded to `isize::MAX`, and null pointers paired with + // nonzero length were rejected above. The borrowed slice is consumed before + // returning across the C ABI. + Ok(unsafe { std::slice::from_raw_parts(ptr, len) }) +} + +fn ffi_utf8(ptr: *const u8, len: usize, label: &str) -> Result<String, ()> { + let bytes = ffi_bytes(ptr, len, label)?; + std::str::from_utf8(bytes).map(str::to_owned).map_err(|_| { + set_last_error(format!("invalid UTF-8 {label}")); + }) +} + +fn copy_to_output( + bytes: &[u8], + output: *mut u8, + output_max: usize, + output_len: *mut usize, + label: &str, +) -> i32 { + if output_len.is_null() { + set_last_error(format!("null {label} length output")); + return -1; + } + if output.is_null() && output_max != 0 { + set_last_error(format!("null {label} output")); + return -1; + } + if output_max > MAX_C_ABI_SLICE_LEN { + set_last_error(format!("{label} output too large")); + return -1; + } + let copy_len = bytes.len().min(output_max); + if copy_len != 0 { + // SAFETY: null output with nonzero capacity was rejected, output_max is + // bounded to `isize::MAX`, and copy_len is capped to both buffers. + 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(); + } + 0 +} + +fn checked_result_indexes(rs: &ResultSet, col: i32, row: i64) -> Option<(usize, usize)> { + let r = usize::try_from(row).ok()?; + let c = usize::try_from(col).ok()?; + if r >= rs.rows.len() || c >= rs.columns.len() || c >= rs.rows[r].len() { + return None; + } + Some((r, c)) +} + // ============================================================ // Database open / close // ============================================================ @@ -85,29 +185,32 @@ fn next_id() -> u64 { pub extern "C" fn jerboa_duckdb_open(path: *const u8, path_len: usize, handle: *mut u64) -> i32 { ffi_wrap(|| { if handle.is_null() { + set_last_error("null duckdb handle output".into()); return -1; } - let conn = if path.is_null() || path_len == 0 { + let conn = if path_len == 0 { Connection::open_in_memory() } else { - let path_bytes = unsafe { std::slice::from_raw_parts(path, path_len) }; - let path_str = match std::str::from_utf8(path_bytes) { - Ok(s) => s, - Err(_) => { - set_last_error("invalid UTF-8 path".into()); - return -1; - } + let path_str = match ffi_utf8(path, path_len, "duckdb path") { + Ok(value) => value, + Err(()) => return -1, }; if path_str == ":memory:" { Connection::open_in_memory() } else { - Connection::open(path_str) + Connection::open(&path_str) } }; match conn { Ok(c) => { let id = next_id(); - DB_STORE.lock().unwrap().insert(id, c); + let mut store = match lock_db_store() { + Some(value) => value, + None => return -1, + }; + store.insert(id, c); + // SAFETY: `handle` was checked non-null above and points to + // caller-owned storage for the C ABI `uint64_t` result slot. unsafe { *handle = id; } @@ -125,7 +228,10 @@ pub extern "C" fn jerboa_duckdb_open(path: *const u8, path_len: usize, handle: * pub extern "C" fn jerboa_duckdb_close(handle: u64) -> i32 { ffi_wrap(|| { // Remove any pending statements for this db - let mut stmts = STMT_STORE.lock().unwrap(); + let mut stmts = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; let to_remove: Vec<u64> = stmts .iter() .filter(|(_, v)| v.db_handle == handle) @@ -136,7 +242,11 @@ pub extern "C" fn jerboa_duckdb_close(handle: u64) -> i32 { } drop(stmts); - match DB_STORE.lock().unwrap().remove(&handle) { + let mut dbs = match lock_db_store() { + Some(value) => value, + None => return -1, + }; + match dbs.remove(&handle) { Some(_) => 0, None => { set_last_error("invalid db handle".into()); @@ -153,18 +263,14 @@ pub extern "C" fn jerboa_duckdb_close(handle: u64) -> i32 { #[no_mangle] pub extern "C" fn jerboa_duckdb_exec(handle: u64, sql: *const u8, sql_len: usize) -> i32 { ffi_wrap(|| { - if sql.is_null() { - return -1; - } - let sql_bytes = unsafe { std::slice::from_raw_parts(sql, sql_len) }; - let sql_str = match std::str::from_utf8(sql_bytes) { - Ok(s) => s, - Err(_) => { - set_last_error("invalid UTF-8 SQL".into()); - return -1; - } + let sql_str = match ffi_utf8(sql, sql_len, "duckdb SQL") { + Ok(value) => value, + Err(()) => return -1, + }; + let store = match lock_db_store() { + Some(value) => value, + None => return -1, }; - let store = DB_STORE.lock().unwrap(); let conn = match store.get(&handle) { Some(c) => c, None => { @@ -172,7 +278,7 @@ pub extern "C" fn jerboa_duckdb_exec(handle: u64, sql: *const u8, sql_len: usize return -1; } }; - match conn.execute_batch(sql_str) { + match conn.execute_batch(&sql_str) { Ok(()) => 0, Err(e) => { set_last_error(format!("duckdb exec: {}", e)); @@ -194,19 +300,19 @@ pub extern "C" fn jerboa_duckdb_prepare( stmt_handle: *mut u64, ) -> i32 { ffi_wrap(|| { - if stmt_handle.is_null() || sql.is_null() { + if stmt_handle.is_null() { + set_last_error("null duckdb statement output".into()); return -1; } - let sql_bytes = unsafe { std::slice::from_raw_parts(sql, sql_len) }; - let sql_str = match std::str::from_utf8(sql_bytes) { - Ok(s) => s.to_string(), - Err(_) => { - set_last_error("invalid UTF-8 SQL".into()); - return -1; - } + let sql_str = match ffi_utf8(sql, sql_len, "duckdb SQL") { + Ok(value) => value, + Err(()) => return -1, }; let parameter_count = { - let store = DB_STORE.lock().unwrap(); + let store = match lock_db_store() { + Some(value) => value, + None => return -1, + }; let conn = match store.get(&db_handle) { Some(conn) => conn, None => { @@ -231,7 +337,11 @@ pub extern "C" fn jerboa_duckdb_prepare( return -1; } let id = next_id(); - STMT_STORE.lock().unwrap().insert( + let mut stmts = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; + stmts.insert( id, PendingStmt { db_handle, @@ -240,6 +350,8 @@ pub extern "C" fn jerboa_duckdb_prepare( binds: Vec::new(), }, ); + // SAFETY: `stmt_handle` was checked non-null above and points to + // caller-owned storage for the C ABI `uint64_t` result slot. unsafe { *stmt_handle = id; } @@ -250,7 +362,10 @@ pub extern "C" fn jerboa_duckdb_prepare( #[no_mangle] pub extern "C" fn jerboa_duckdb_bind_int(stmt_handle: u64, index: i32, value: i64) -> i32 { ffi_wrap(|| { - let mut store = STMT_STORE.lock().unwrap(); + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; let entry = match store.get_mut(&stmt_handle) { Some(e) => e, None => { @@ -271,7 +386,10 @@ pub extern "C" fn jerboa_duckdb_bind_int(stmt_handle: u64, index: i32, value: i6 #[no_mangle] pub extern "C" fn jerboa_duckdb_bind_double(stmt_handle: u64, index: i32, value: f64) -> i32 { ffi_wrap(|| { - let mut store = STMT_STORE.lock().unwrap(); + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; let entry = match store.get_mut(&stmt_handle) { Some(e) => e, None => { @@ -297,22 +415,14 @@ pub extern "C" fn jerboa_duckdb_bind_text( text_len: usize, ) -> i32 { ffi_wrap(|| { - if text.is_null() && text_len > 0 { - return -1; - } - let s = if text_len == 0 { - String::new() - } else { - let bytes = unsafe { std::slice::from_raw_parts(text, text_len) }; - match std::str::from_utf8(bytes) { - Ok(s) => s.to_string(), - Err(_) => { - set_last_error("invalid UTF-8 text".into()); - return -1; - } - } + let s = match ffi_utf8(text, text_len, "duckdb text") { + Ok(value) => value, + Err(()) => return -1, + }; + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, }; - let mut store = STMT_STORE.lock().unwrap(); let entry = match store.get_mut(&stmt_handle) { Some(e) => e, None => { @@ -338,12 +448,14 @@ pub extern "C" fn jerboa_duckdb_bind_blob( data_len: usize, ) -> i32 { ffi_wrap(|| { - let blob = if data.is_null() || data_len == 0 { - Vec::new() - } else { - unsafe { std::slice::from_raw_parts(data, data_len) }.to_vec() + let blob = match ffi_bytes(data, data_len, "duckdb blob") { + Ok(value) => value.to_vec(), + Err(()) => return -1, + }; + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, }; - let mut store = STMT_STORE.lock().unwrap(); let entry = match store.get_mut(&stmt_handle) { Some(e) => e, None => { @@ -364,7 +476,10 @@ pub extern "C" fn jerboa_duckdb_bind_blob( #[no_mangle] pub extern "C" fn jerboa_duckdb_bind_null(stmt_handle: u64, index: i32) -> i32 { ffi_wrap(|| { - let mut store = STMT_STORE.lock().unwrap(); + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; let entry = match store.get_mut(&stmt_handle) { Some(e) => e, None => { @@ -385,7 +500,10 @@ pub extern "C" fn jerboa_duckdb_bind_null(stmt_handle: u64, index: i32) -> i32 { #[no_mangle] pub extern "C" fn jerboa_duckdb_bind_bool(stmt_handle: u64, index: i32, value: i32) -> i32 { ffi_wrap(|| { - let mut store = STMT_STORE.lock().unwrap(); + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; let entry = match store.get_mut(&stmt_handle) { Some(e) => e, None => { @@ -424,6 +542,53 @@ mod security_tests { assert_eq!(jerboa_duckdb_finalize(stmt), 0); assert_eq!(jerboa_duckdb_close(db), 0); } + + #[test] + fn null_nonzero_inputs_are_rejected() { + let mut db = 0; + assert_eq!(jerboa_duckdb_open(std::ptr::null(), 1, &mut db), -1); + assert_eq!(jerboa_duckdb_exec(u64::MAX, std::ptr::null(), 1), -1); + assert_eq!( + jerboa_duckdb_bind_text(u64::MAX, 1, std::ptr::null(), 1), + -1 + ); + assert_eq!( + jerboa_duckdb_bind_blob(u64::MAX, 1, std::ptr::null(), 1), + -1 + ); + } + + #[test] + fn output_buffers_are_validated() { + let mut len = 0; + assert_eq!(jerboa_duckdb_version(std::ptr::null_mut(), 1, &mut len), -1); + assert_eq!(jerboa_duckdb_version(std::ptr::null_mut(), 0, &mut len), 0); + assert_eq!(len, "duckdb-rs".len()); + assert_eq!( + jerboa_duckdb_version(std::ptr::null_mut(), 0, std::ptr::null_mut()), + -1 + ); + } + + #[test] + fn result_indexes_reject_negative_values() { + let handle = next_id(); + let result = ResultSet { + columns: vec!["value".to_string()], + column_types: vec![1], + rows: vec![vec![CellValue::Int(42)]], + }; + lock_result_store().unwrap().insert(handle, result); + + assert_eq!(jerboa_duckdb_column_type(handle, -1), -1); + assert_eq!(jerboa_duckdb_value_is_null(handle, -1, 0), -1); + assert_eq!(jerboa_duckdb_value_is_null(handle, 0, -1), -1); + assert_eq!(jerboa_duckdb_value_int(handle, -1, 0), 0); + assert_eq!(jerboa_duckdb_value_double(handle, 0, -1), 0.0); + assert_eq!(jerboa_duckdb_value_bool(handle, -1, 0), 0); + + assert_eq!(jerboa_duckdb_free_result(handle), 0); + } } /// Execute a prepared statement and materialize the result set. @@ -433,12 +598,16 @@ mod security_tests { pub extern "C" fn jerboa_duckdb_execute(stmt_handle: u64, result_handle_out: *mut u64) -> i32 { ffi_wrap(|| { if result_handle_out.is_null() { + set_last_error("null duckdb result output".into()); return -1; } // Extract the pending statement let pending = { - let store = STMT_STORE.lock().unwrap(); + let store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; match store.get(&stmt_handle) { Some(p) => PendingStmt { db_handle: p.db_handle, @@ -454,7 +623,10 @@ pub extern "C" fn jerboa_duckdb_execute(stmt_handle: u64, result_handle_out: *mu }; // Execute against the connection - let db_store = DB_STORE.lock().unwrap(); + let db_store = match lock_db_store() { + Some(value) => value, + None => return -1, + }; let conn = match db_store.get(&pending.db_handle) { Some(c) => c, None => { @@ -538,10 +710,14 @@ pub extern "C" fn jerboa_duckdb_execute(stmt_handle: u64, result_handle_out: *mu }; let rid = next_id(); - RESULT_STORE.lock().unwrap().insert(rid, result_set); - unsafe { - *result_handle_out = rid; - } + let mut results = match lock_result_store() { + Some(value) => value, + None => return -1, + }; + results.insert(rid, result_set); + // SAFETY: `result_handle_out` was checked non-null above and points to + // caller-owned storage for the C ABI `uint64_t` result slot. + unsafe { *result_handle_out = rid } 0 }) } @@ -653,7 +829,10 @@ fn extract_cell(row: &duckdb::Row<'_>, idx: usize, column_types: &mut Vec<i32>) #[no_mangle] pub extern "C" fn jerboa_duckdb_nrows(result_handle: u64) -> i64 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; match store.get(&result_handle) { Some(r) => r.rows.len() as i64, None => -1, @@ -662,7 +841,10 @@ pub extern "C" fn jerboa_duckdb_nrows(result_handle: u64) -> i64 { #[no_mangle] pub extern "C" fn jerboa_duckdb_ncols(result_handle: u64) -> i64 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; match store.get(&result_handle) { Some(r) => r.columns.len() as i64, None => -1, @@ -678,10 +860,10 @@ pub extern "C" fn jerboa_duckdb_column_name( output_len: *mut usize, ) -> i32 { ffi_wrap(|| { - if output.is_null() || output_len.is_null() { - return -1; - } - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; let rs = match store.get(&result_handle) { Some(r) => r, None => { @@ -689,29 +871,40 @@ pub extern "C" fn jerboa_duckdb_column_name( return -1; } }; - let idx = col as usize; + let idx = match usize::try_from(col) { + Ok(value) => value, + Err(_) => { + set_last_error("column index out of range".into()); + return -1; + } + }; if idx >= rs.columns.len() { set_last_error("column index out of range".into()); return -1; } - let bytes = rs.columns[idx].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]); - unsafe { - *output_len = bytes.len(); - } - 0 + copy_to_output( + rs.columns[idx].as_bytes(), + output, + output_max, + output_len, + "duckdb column name", + ) }) } /// Column type: 1=INTEGER, 2=FLOAT, 3=TEXT, 4=BLOB, 5=NULL, 6=BOOLEAN #[no_mangle] pub extern "C" fn jerboa_duckdb_column_type(result_handle: u64, col: i32) -> i32 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; match store.get(&result_handle) { Some(rs) => { - let idx = col as usize; + let idx = match usize::try_from(col) { + Ok(value) => value, + Err(_) => return -1, + }; if idx >= rs.column_types.len() { return -1; } @@ -723,14 +916,16 @@ pub extern "C" fn jerboa_duckdb_column_type(result_handle: u64, col: i32) -> i32 #[no_mangle] pub extern "C" fn jerboa_duckdb_value_is_null(result_handle: u64, col: i32, row: i64) -> i32 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; match store.get(&result_handle) { Some(rs) => { - let r = row as usize; - let c = col as usize; - if r >= rs.rows.len() || c >= rs.columns.len() { - return -1; - } + let (r, c) = match checked_result_indexes(rs, col, row) { + Some(value) => value, + None => return -1, + }; match &rs.rows[r][c] { CellValue::Null => 1, _ => 0, @@ -742,14 +937,16 @@ pub extern "C" fn jerboa_duckdb_value_is_null(result_handle: u64, col: i32, row: #[no_mangle] pub extern "C" fn jerboa_duckdb_value_int(result_handle: u64, col: i32, row: i64) -> i64 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return 0, + }; match store.get(&result_handle) { Some(rs) => { - let r = row as usize; - let c = col as usize; - if r >= rs.rows.len() || c >= rs.columns.len() { - return 0; - } + let (r, c) = match checked_result_indexes(rs, col, row) { + Some(value) => value, + None => return 0, + }; match &rs.rows[r][c] { CellValue::Int(v) => *v, CellValue::Bool(b) => { @@ -768,14 +965,16 @@ pub extern "C" fn jerboa_duckdb_value_int(result_handle: u64, col: i32, row: i64 #[no_mangle] pub extern "C" fn jerboa_duckdb_value_double(result_handle: u64, col: i32, row: i64) -> f64 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return 0.0, + }; match store.get(&result_handle) { Some(rs) => { - let r = row as usize; - let c = col as usize; - if r >= rs.rows.len() || c >= rs.columns.len() { - return 0.0; - } + let (r, c) = match checked_result_indexes(rs, col, row) { + Some(value) => value, + None => return 0.0, + }; match &rs.rows[r][c] { CellValue::Double(v) => *v, CellValue::Int(v) => *v as f64, @@ -788,14 +987,16 @@ pub extern "C" fn jerboa_duckdb_value_double(result_handle: u64, col: i32, row: #[no_mangle] pub extern "C" fn jerboa_duckdb_value_bool(result_handle: u64, col: i32, row: i64) -> i32 { - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return 0, + }; match store.get(&result_handle) { Some(rs) => { - let r = row as usize; - let c = col as usize; - if r >= rs.rows.len() || c >= rs.columns.len() { - return 0; - } + let (r, c) = match checked_result_indexes(rs, col, row) { + Some(value) => value, + None => return 0, + }; match &rs.rows[r][c] { CellValue::Bool(b) => { if *b { @@ -828,74 +1029,39 @@ pub extern "C" fn jerboa_duckdb_value_text( output_len: *mut usize, ) -> i32 { ffi_wrap(|| { - if output.is_null() || output_len.is_null() { - return -1; - } - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; let rs = match store.get(&result_handle) { Some(r) => r, None => return -1, }; - let r = row as usize; - let c = col as usize; - if r >= rs.rows.len() || c >= rs.columns.len() { - return -1; - } - let text = match &rs.rows[r][c] { + let (r, c) = match checked_result_indexes(rs, col, row) { + Some(value) => value, + None => return -1, + }; + let rendered; + let bytes = match &rs.rows[r][c] { CellValue::Text(s) => s.as_bytes(), CellValue::Int(v) => { - let s = v.to_string(); - 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]); - unsafe { - *output_len = bytes.len(); - } - return 0; + rendered = v.to_string(); + rendered.as_bytes() } CellValue::Double(v) => { - let s = v.to_string(); - 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]); - unsafe { - *output_len = bytes.len(); - } - return 0; + rendered = v.to_string(); + rendered.as_bytes() } CellValue::Bool(b) => { - let s = if *b { "true" } else { "false" }; - 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]); - unsafe { - *output_len = bytes.len(); - } - return 0; - } - CellValue::Null => { - unsafe { - *output_len = 0; - } - return 0; - } - CellValue::Blob(_) => { - unsafe { - *output_len = 0; + if *b { + &b"true"[..] + } else { + &b"false"[..] } - return 0; } + CellValue::Null | CellValue::Blob(_) => &[], }; - let copy_len = text.len().min(output_max); - let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; - out[..copy_len].copy_from_slice(&text[..copy_len]); - unsafe { - *output_len = text.len(); - } - 0 + copy_to_output(bytes, output, output_max, output_len, "duckdb text value") }) } @@ -909,41 +1075,27 @@ pub extern "C" fn jerboa_duckdb_value_blob( output_len: *mut usize, ) -> i32 { ffi_wrap(|| { - if output.is_null() || output_len.is_null() { - return -1; - } - let store = RESULT_STORE.lock().unwrap(); + let store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; let rs = match store.get(&result_handle) { Some(r) => r, None => return -1, }; - let r = row as usize; - let c = col as usize; - if r >= rs.rows.len() || c >= rs.columns.len() { - return -1; - } + let (r, c) = match checked_result_indexes(rs, col, row) { + Some(value) => value, + None => return -1, + }; let blob = match &rs.rows[r][c] { CellValue::Blob(b) => b.as_slice(), - CellValue::Null => { - unsafe { - *output_len = 0; - } - return 0; - } - _ => { - unsafe { - *output_len = 0; - } - return 0; - } + CellValue::Null + | CellValue::Int(_) + | CellValue::Double(_) + | CellValue::Text(_) + | CellValue::Bool(_) => &[], }; - let copy_len = blob.len().min(output_max); - let out = unsafe { std::slice::from_raw_parts_mut(output, output_max) }; - out[..copy_len].copy_from_slice(&blob[..copy_len]); - unsafe { - *output_len = blob.len(); - } - 0 + copy_to_output(blob, output, output_max, output_len, "duckdb blob value") }) } @@ -953,24 +1105,34 @@ pub extern "C" fn jerboa_duckdb_value_blob( #[no_mangle] pub extern "C" fn jerboa_duckdb_free_result(result_handle: u64) -> i32 { - ffi_wrap( - || match RESULT_STORE.lock().unwrap().remove(&result_handle) { + ffi_wrap(|| { + let mut store = match lock_result_store() { + Some(value) => value, + None => return -1, + }; + match store.remove(&result_handle) { Some(_) => 0, None => { set_last_error("invalid result handle".into()); -1 } - }, - ) + } + }) } #[no_mangle] pub extern "C" fn jerboa_duckdb_finalize(stmt_handle: u64) -> i32 { - ffi_wrap(|| match STMT_STORE.lock().unwrap().remove(&stmt_handle) { - Some(_) => 0, - None => { - set_last_error("invalid stmt handle".into()); - -1 + ffi_wrap(|| { + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; + match store.remove(&stmt_handle) { + Some(_) => 0, + None => { + set_last_error("invalid stmt handle".into()); + -1 + } } }) } @@ -979,7 +1141,10 @@ pub extern "C" fn jerboa_duckdb_finalize(stmt_handle: u64) -> i32 { #[no_mangle] pub extern "C" fn jerboa_duckdb_reset(stmt_handle: u64) -> i32 { ffi_wrap(|| { - let mut store = STMT_STORE.lock().unwrap(); + let mut store = match lock_stmt_store() { + Some(value) => value, + None => return -1, + }; match store.get_mut(&stmt_handle) { Some(entry) => { entry.binds.clear(); @@ -1005,17 +1170,13 @@ pub extern "C" fn jerboa_duckdb_version( output_len: *mut usize, ) -> i32 { ffi_wrap(|| { - if output.is_null() || output_len.is_null() { - return -1; - } let version = "duckdb-rs"; - let bytes = version.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]); - unsafe { - *output_len = bytes.len();