security: add ffi audit per-site verdicts
Jaime Fournier <jaimef@linbsd.org>
8e16f3debfaa14aeda3f1a6208553d04c25bdd80
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -24,22 +24,27 @@ Latest summary from `tools/ffi-audit-report.ss`: (schema "jerboa.ffi-audit/1") (scheme (file-count 1202) - (site-count 956) - (foreign-procedure 855) - (load-shared-object 87) + (site-count 904) + (foreign-procedure 810) + (load-shared-object 80) (define-ftype 7) (foreign-callable 7) - (pointer-sites 334) + (pointer-sites 333) (width-sensitive-sites 180) - (blocking-candidates 130) - (blocking-without-collect-safe 68)) + (blocking-candidates 128) + (blocking-without-collect-safe 66) + (hazard-site-count 416) + (blocking-review-sites 66) + (safety-review-sites 350)) (native (rust-file-count 29) (c-file-count 5) (no-mangle-exports 231) - (unsafe-sites 432) - (unsafe-sites-with-nearby-safety-comment 2) - (unsafe-sites-without-nearby-safety-comment 430)) + (unsafe-sites 438) + (unsafe-sites-with-nearby-safety-comment 5) + (unsafe-sites-without-nearby-safety-comment 433) + (unsafe-comment-review-sites 433) + (export-review-sites 231)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-pending-cve-gate))) (verdict needs-per-binding-review)) @@ -50,7 +55,12 @@ Latest summary from `tools/ffi-audit-report.ss`: K3-P1-01 is now started and reproducibly inventoried, but not complete. The current release gate has a machine-readable count of Scheme FFI sites, native C/Rust files, Rust C ABI exports, pointer/width-sensitive bindings, blocking -candidate calls, and unsafe Rust sites. +candidate calls, and unsafe Rust sites. The full report appends a provisional +`hazards` list and `verdict` to every Scheme and Rust site. Scheme verdicts +separate blocking calls missing `__collect_safe`, pointer/GC/ownership or +integer-width review, and lower-risk API review. Rust verdicts separate unsafe +sites missing nearby `SAFETY:` comments from C ABI exports that still need +caller reconciliation. The native Rust crate now denies `unsafe_op_in_unsafe_fn`, so an unsafe function body no longer grants implicit permission for unsafe operations. Each @@ -59,7 +69,8 @@ unsafe operation still has to live inside an explicit `unsafe { ... }` block. Remaining work before closing K3-P1-01: - Review every `foreign-procedure`, `foreign-callable`, and `define-ftype` - binding for null returns, bounds derivation, ownership, and GC safety. + binding using the per-site `hazards` / `verdict` fields for null returns, + bounds derivation, ownership, and GC safety. - Convert blocking candidates without `__collect_safe` or justify them as nonblocking. - Add targeted scanner rules for null-return checks, integer width confusion, --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -600,11 +600,14 @@ not started." - **Status:** started. `docs/ffi-audit.md` and `tools/ffi-audit-report.ss` now provide a reproducible inventory for Scheme FFI sites, Rust C ABI exports, pointer/width-sensitive bindings, blocking candidates, and unsafe - Rust sites. `jerboa-native-rs` now denies `unsafe_op_in_unsafe_fn`, so unsafe - function bodies do not implicitly permit unsafe operations. Remaining work: - per-binding verdicts, scanner rules for null/width/bounds/GC-safety hazards, - unsafe invariant comments, export shrinking/justification, and the - `vendor/jsqlite` CVE/replacement decision. + Rust sites. The reporter now skips comment-only mentions, emits per-site + `hazards` and provisional `verdict` fields in `--full` output, and summarizes + blocking review, pointer/width safety review, unsafe-comment review, and + export-review counts. `jerboa-native-rs` now denies + `unsafe_op_in_unsafe_fn`, so unsafe function bodies do not implicitly permit + unsafe operations. Remaining work: scanner rules for null/width/bounds/ + GC-safety hazards, unsafe invariant comments, export shrinking/ + justification, and the `vendor/jsqlite` CVE/replacement decision. ### K3-P1-02 — TOCTOU-safe filesystem capability checks **Serves:** G2. **Effort:** 1 week. --- a/tools/ffi-audit-report.ss +++ b/tools/ffi-audit-report.ss @@ -46,6 +46,36 @@ (or (string-contains* line (car xs)) (loop (cdr xs)))))) +(define (blank-or-comment-line? line comment-char) + (let ([n (string-length line)]) + (let loop ([i 0]) + (cond + [(= i n) #t] + [(or (char=? (string-ref line i) #\space) + (char=? (string-ref line i) #\tab)) + (loop (+ i 1))] + [(char=? (string-ref line i) comment-char) #t] + [else #f])))) + +(define (rust-comment-line? line) + (let ([n (string-length line)]) + (let loop ([i 0]) + (cond + [(= i n) #t] + [(or (char=? (string-ref line i) #\space) + (char=? (string-ref line i) #\tab)) + (loop (+ i 1))] + [(and (< (+ i 1) n) + (char=? (string-ref line i) #\/) + (char=? (string-ref line (+ i 1)) #\/)) + #t] + [(and (< (+ i 1) n) + (char=? (string-ref line i) #\/) + (char=? (string-ref line (+ i 1)) #\*)) + #t] + [(char=? (string-ref line i) #\*) #t] + [else #f])))) + (define (string-prefix? prefix str) (let ([plen (string-length prefix)] [len (string-length str)]) @@ -101,6 +131,7 @@ (define (scheme-kind line) (cond + [(blank-or-comment-line? line #\;) #f] [(string-contains* line "foreign-procedure") 'foreign-procedure] [(string-contains* line "load-shared-object") 'load-shared-object] [(string-contains* line "define-ftype") 'define-ftype] @@ -150,9 +181,12 @@ [(null? rest) (reverse out)] [else (let* ([line (car rest)] - [no-mangle? (or (string-contains* line "#[no_mangle]") - (string-contains* line "no_mangle"))] - [unsafe? (string-contains* line "unsafe")] + [comment? (rust-comment-line? line)] + [no-mangle? (and (not comment?) + (or (string-contains* line "#[no_mangle]") + (string-contains* line "no_mangle")))] + [unsafe? (and (not comment?) + (string-contains* line "unsafe"))] [out* (if no-mangle? (cons `(rust-site (path ,path) (line ,(+ zero 1)) (kind no-mangle) (snippet ,line)) @@ -180,6 +214,53 @@ (define (site-bool site key) (cadr (assq key (cdr site)))) +(define (scheme-site-hazards site) + (let ([out '()]) + (when (site-bool site 'pointer-args) + (set! out (cons 'pointer-ownership-gc-safety out))) + (when (site-bool site 'width-sensitive) + (set! out (cons 'integer-width-bounds out))) + (when (and (site-bool site 'blocking-candidate) + (not (site-bool site 'collect-safe))) + (set! out (cons 'blocking-without-collect-safe out))) + (reverse out))) + +(define (scheme-site-verdict site) + (let ([hazards (scheme-site-hazards site)]) + (cond + [(memq 'blocking-without-collect-safe hazards) 'needs-blocking-review] + [(or (memq 'pointer-ownership-gc-safety hazards) + (memq 'integer-width-bounds hazards)) + 'needs-safety-review] + [else 'needs-api-review]))) + +(define (annotate-scheme-site site) + (append site + `((hazards ,(scheme-site-hazards site)) + (verdict ,(scheme-site-verdict site))))) + +(define (rust-site-hazards site) + (cond + [(eq? (site-kind site) 'unsafe) + (if (site-bool site 'safety-comment) + '(unsafe-reviewed-comment-present) + '(unsafe-missing-safety-comment))] + [(eq? (site-kind site) 'no-mangle) '(c-abi-export-needs-caller-check)] + [else '()])) + +(define (rust-site-verdict site) + (cond + [(memq 'unsafe-missing-safety-comment (rust-site-hazards site)) + 'needs-safety-comment] + [(memq 'c-abi-export-needs-caller-check (rust-site-hazards site)) + 'needs-export-review] + [else 'reviewed-note-present])) + +(define (annotate-rust-site site) + (append site + `((hazards ,(rust-site-hazards site)) + (verdict ,(rust-site-verdict site))))) + (define (kind-count sites kind) (count-if (lambda (site) (eq? (site-kind site) kind)) sites)) @@ -195,6 +276,8 @@ (string-suffix? ".h" p))))] [scheme (apply append (map scheme-sites scheme-files))] [rust (apply append (map rust-sites rust-files))] + [scheme* (map annotate-scheme-site scheme)] + [rust* (map annotate-rust-site rust)] [unsafe-count (kind-count rust 'unsafe)] [unsafe-commented (count-if (lambda (site) @@ -218,6 +301,16 @@ ,(count-if (lambda (s) (and (site-bool s 'blocking-candidate) (not (site-bool s 'collect-safe)))) + scheme)) + (hazard-site-count + ,(count-if (lambda (s) (pair? (scheme-site-hazards s))) scheme)) + (blocking-review-sites + ,(count-if (lambda (s) + (eq? (scheme-site-verdict s) 'needs-blocking-review)) + scheme)) + (safety-review-sites + ,(count-if (lambda (s) + (eq? (scheme-site-verdict s) 'needs-safety-review)) scheme))) (native (rust-file-count ,(length rust-files)) @@ -226,12 +319,20 @@ (unsafe-sites ,unsafe-count) (unsafe-sites-with-nearby-safety-comment ,unsafe-commented) (unsafe-sites-without-nearby-safety-comment - ,(- unsafe-count unsafe-commented))) + ,(- unsafe-count unsafe-commented)) + (unsafe-comment-review-sites + ,(count-if (lambda (s) + (eq? (rust-site-verdict s) 'needs-safety-comment)) + rust)) + (export-review-sites + ,(count-if (lambda (s) + (eq? (rust-site-verdict s) 'needs-export-review)) + rust))) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-pending-cve-gate))) (verdict needs-per-binding-review) ,@(if full? - `((scheme-sites ,scheme) (rust-sites ,rust)) + `((scheme-sites ,scheme*) (rust-sites ,rust*)) '()))) (newline)))