Save security review discoveries
ober
a884aef3b4c437ee1b0cf949a7cee136c7ae2c36
--- a/data/cookbooks.sexp +++ b/data/cookbooks.sexp @@ -5955,4 +5955,36 @@ "For Jerboa verifier or contract scripts, prefer the prelude-provided `exit` over importing `(chezscheme)` just for process termination or binding `(foreign-procedure \"_exit\" ...)`. Direct Chez imports weaken capability boundaries, and top-level foreign `_exit` bindings can fail during static/MCP verification when the symbol is unavailable.") ("tags" "prelude" "exit" "verifier" "contract" "chezscheme" "ffi") - ("title" . "Use prelude exit for verifier finish helpers"))) + ("title" . "Use prelude exit for verifier finish helpers")) + (("code" + . + "(import (chezscheme))\n\n(define (write-private-file path contents)\n ;; call-with-output-file still respects the process umask; tighten after write.\n (call-with-output-file path\n (lambda (p) (display contents p))\n 'replace)\n (chmod path #o600)\n path)\n\n(write-private-file \"secret.txt\" \"token=redacted\\n\")") ("id" . "chmod-private-file-permissions") + ("imports" "(chezscheme)") + ("notes" + . + "Use Chez Scheme's built-in chmod instead of a raw (foreign-procedure \"chmod\" ...). The built-in path avoids missing-symbol failures in static binaries and keeps the source portable across Jerboa projects. Tighten permissions after creating/replacing the file because the process umask may otherwise leave secrets world-readable.") + ("tags" "chmod" "permissions" "secret-file" "static-binary" + "chezscheme" "filesystem") + ("title" . "Set private file permissions with Chez chmod")) + (("code" + . + "(import (chezscheme))\n\n;; Make process/libc symbols visible for foreign-procedure lookups.\n(load-shared-object #f)\n\n(define c-waitpid\n (foreign-procedure \"waitpid\" (int void* int) int))\n\n(define (waitpid-status pid)\n (let ([status-ptr (foreign-alloc (foreign-sizeof 'int))])\n (dynamic-wind\n (lambda () (void))\n (lambda ()\n (let ([rc (c-waitpid pid status-ptr 0)])\n (values rc (foreign-ref 'int status-ptr 0))))\n (lambda () (foreign-free status-ptr)))))") ("id" . "ffi-out-parameter-foreign-alloc") + ("imports" "(chezscheme)") + ("notes" + . + "For C APIs that write through an out-parameter, allocate C memory with foreign-alloc, declare the parameter as void*, read it with foreign-ref, and free it in dynamic-wind. Do not manufacture a pointer to a Scheme bytevector with object-address arithmetic; moving/runtime layout details make that unsafe and non-portable. On macOS and other dynamic builds, call (load-shared-object #f) before resolving process/libc symbols directly; in static binaries, register the needed symbols instead.") + ("tags" "ffi" "foreign-alloc" "foreign-ref" "dynamic-wind" + "out-parameter" "waitpid") + ("title" . "Use foreign-alloc for C out-parameters")) + (("code" + . + "(import (chezscheme))\n\n(define (require-bytes who b off len)\n (unless (and (bytevector? b)\n (integer? off) (integer? len)\n (>= off 0) (>= len 0)\n (<= (+ off len) (bytevector-length b)))\n (error who \"truncated payload\" off len)))\n\n(define (u16be-ref b off)\n (require-bytes 'u16be-ref b off 2)\n (+ (* 256 (bytevector-u8-ref b off))\n (bytevector-u8-ref b (+ off 1))))\n\n(define (read-u16be-field b off)\n (let* ([len (u16be-ref b off)]\n [data-off (+ off 2)]\n [next-off (+ data-off len)])\n (require-bytes 'read-u16be-field b data-off len)\n (values (subbytevector b data-off next-off) next-off)))\n\n(define (parse-one-field b)\n (let-values ([(payload next-off) (read-u16be-field b 0)])\n (unless (= next-off (bytevector-length b))\n (error 'parse-one-field \"trailing payload bytes\"))\n payload))") ("id" . "checked-bytevector-length-parser") + ("imports" "(chezscheme)") + ("notes" + . + "Always check that both the length field and the length-delimited payload fit before bytevector-u8-ref or subbytevector. Also reject trailing bytes when the wire format requires exact consumption. This turns malformed or truncated input into a Scheme error instead of an out-of-range access or silent parse confusion.") + ("tags" "bytevector" "binary-parser" "bounds-check" + "length-prefix" "deserialize" "security") + ("title" + . + "Bounds-check length-prefixed bytevector parsers"))) --- a/data/features.sexp +++ b/data/features.sexp @@ -2478,4 +2478,40 @@ ("use_case" . "Training and evaluating local models on GUI/game construction incrementally before attempting a full Tetris clone.") + ("votes" . 0)) + (("description" + . + "Add a Jerboa MCP tool that accepts a list of project paths or a glob such as ~/mine/jerboa* and runs the standard checks per repository: git status, balance/syntax checks for changed Scheme files, security scan on changed files, known build/test targets, and a compact final matrix. It should preserve dirty-tree boundaries and clearly separate pre-existing local changes from files touched during the current session.") + ("estimated_token_reduction" + . + "~2000-4000 tokens per large multi-repo pass") + ("example_scenario" + . + "A security review across ~/mine/jerboa* required dozens of repeated git status, diff, build, test, and push checks. A single multi-repo audit could have summarized clean/dirty/ahead/blocked state and reduced manual orchestration.") + ("id" . "multi-repo-jerboa-audit") ("impact" . "high") + ("status" . "open") + ("tags" "multi-repo" "audit" "status" "build-test" + "security") + ("title" . "Audit multiple Jerboa repos in one run") + ("use_case" + . + "Large Jerboa review/fix passes that touch many sibling repositories and need reliable final verification before commits and pushes.") + ("votes" . 0)) + (("description" + . + "Teach the security scanner to treat a foreign-alloc allocation as freed when the pointer is released in a dynamic-wind after thunk. The current foreign-alloc-no-free rule reported a leak on code that allocates an int pointer, uses it, and frees it in the cleanup thunk.") + ("estimated_token_reduction" + . + "~300-600 tokens per FFI review by avoiding manual false-positive explanation") + ("example_scenario" + . + "The fixed anti-debug waitpid code uses foreign-alloc for a status int and foreign-free in dynamic-wind, but the scanner still flagged foreign-alloc-no-free.") + ("id" . "foreign-alloc-dynamic-wind-free-recognition") + ("impact" . "medium") ("status" . "open") + ("tags" "security-scan" "ffi" "foreign-alloc" "dynamic-wind" + "false-positive") + ("title" . "Recognize dynamic-wind foreign-free cleanup") + ("use_case" + . + "Reviewing Scheme FFI code that safely allocates temporary C memory for out-parameters or buffers.") ("votes" . 0))) --- a/data/security-rules.sexp +++ b/data/security-rules.sexp @@ -945,4 +945,36 @@ . "foreign-procedure\\s+\"[^\"]*(read|write|recv|send|poll|select|accept|connect|sleep|wait|flush|peek|present)") ("scope" . "ffi-boundary") + ("severity" . "high")) + (("id" . "scheme-bytevector-object-address-ffi-pointer") + ("message" + . + "Do not pass Scheme bytevector storage to C by manufacturing pointers with object-address arithmetic. Allocate C memory with foreign-alloc, pass it as void*, read with foreign-ref, and release it with foreign-free in dynamic-wind.") + ("pattern" . "#%\\$object-address|bytevector->uptr") + ("scope" . "ffi-boundary") + ("severity" . "high")) + (("id" . "rust-ffi-box-from-raw-borrow-leak") + ("message" + . + "Do not use Box::from_raw to borrow an FFI-owned handle unless the function is taking ownership. Box::leak(Box::from_raw(ptr)) hides ownership bugs and leaks or corrupts ownership. Use &*ptr or &mut *ptr after explicit null checks for borrowed handles.") + ("pattern" . "Box::leak\\s*\\(\\s*Box::from_raw") + ("scope" . "ffi-boundary") + ("severity" . "high")) + (("id" . "c-shim-static-mutable-buffer") + ("message" + . + "Mutable process-global buffers in C shims are shared by all Scheme threads and calls. They can race, leak data between callers, and corrupt results. Prefer caller-owned buffers, heap-allocated result handles, or static __thread storage when a compatibility buffer is unavoidable.") + ("pattern" + . + "static\\s+(char|unsigned\\s+char|uint8_t|struct\\s+[A-Za-z_][A-Za-z0-9_]*)\\s+[A-Za-z_][A-Za-z0-9_]*\\s*(\\[|;)") + ("scope" . "c-shim") + ("severity" . "medium")) + (("id" . "scheme-open-process-composed-shell-command") + ("message" + . + "Building an open-process-ports command string with string-append, format, or string-join can turn user-controlled values into shell syntax. Prefer an argument-vector process API when available; otherwise quote every shell word with a single well-reviewed helper and avoid passing secrets in argv.") + ("pattern" + . + "open-process-ports\\s*\\(\\s*(string-append|format|string-join)") + ("scope" . "scheme") ("severity" . "high")))