data: add P0 prevention artifacts (10 scanner patterns, 8 anti-patterns, 5 recipes)
ober
a46cf489adda3e79b17a2ee74b9fb1b6ac808696
--- a/data/anti-patterns.sexp +++ b/data/anti-patterns.sexp @@ -4806,4 +4806,115 @@ ("title" . "Allocating/reading a size taken from untrusted input") + ("tools" "jerboa_howto" "jerboa_security_scan")) + (("advice" + . + "Use the Result type (ok/err/try-result/and-then/->?) so an error is distinguishable from absent/no-match. Fail closed: a limit/parse/read error must raise or return a distinct error sentinel, never the benign value. Add a negative regression test that triggers the error path and asserts the control still blocks.") + ("avoid" + . + "Returning #f (or '()) for BOTH a genuine no-match AND a limit/error/exception. An attacker triggers the error to bypass a control: pcre2 MATCHLIMIT/DEPTHLIMIT/NOMEMORY -> #f defeated a defender pattern; top truncated ps output -> empty list hid every process; signal wrong-passphrase -> #f silently disabled encrypted logging; gitsafe file-read error -> reported clean.") + ("id" . "error-as-false-fail-open") + ("kinds" "security" "correctness") ("pattern" . "") + ("severity" . "high") + ("tags" "fail-open" "result-type" "error-handling" "limit" + "no-match" "fail-closed") + ("title" . "Mapping a limit/error to #f (fail-open)") + ("tools" "jerboa_howto" "jerboa_security_scan")) + (("advice" + . + "Carry long-lived secrets as bytevectors end-to-end; zero with bytevector-fill! after use; document the residual risk where a string is unavoidable. Read secrets from /dev/tty or an fd, not the environment (env is inherited by every child and visible in /proc). Scrub secret env vars from child environments before spawning.") + ("avoid" + . + "Promoting a passphrase/key/token/plaintext to a Scheme string. Strings are immutable and GC-copied, so they CANNOT be zeroed — this undoes Rust zeroize and leaves secrets in the heap and core dumps. Seen across pgp, proton-bridge, yubikey, signal, drive, browser.") + ("id" . "secrets-in-immutable-strings") ("kinds" "security") + ("pattern" . "") ("severity" . "high") + ("tags" "secrets" "zeroize" "bytevector" "string" "gc" + "core-dump" "env") + ("title" + . + "Holding secrets in immutable Scheme strings (unzeroable)") + ("tools" "jerboa_howto")) + (("advice" + . + "Use defstruct/defrecord with named accessors; use define-enum instead of magic numbers; use match to destructure. This makes an inserted/removed field a compile-time error instead of a silent runtime misindex.") + ("avoid" + . + "Hand-rolled records as vectors/lists accessed by vector-ref/list-ref with magic indices. Two files disagreeing on arity (or an inserted field) silently shifts every accessor — a recurring off-by-one source (wafter field-spec parser built 9 / engine read 7; https config vectors; fuse inode 13-tuple; asm 46 accessors; top history/snapshot; imagesite app-state; qt handler record).") + ("id" . "positional-records-off-by-one") + ("kinds" "correctness" "security") ("pattern" . "") + ("severity" . "medium") + ("tags" "defstruct" "positional" "vector-ref" "list-ref" + "off-by-one" "record") + ("title" + . + "Positional vector/list records (off-by-one magnet)") + ("tools" "jerboa_howto")) + (("advice" + . + "Use with-lock for mutexes, with-resource/call-with-port for ports/fds, errdefer for error-only cleanup. Caveat: core with-lock acquires in the before-thunk (unsafe under call/cc) — acquire once before dynamic-wind if continuations are in play.") + ("avoid" + . + "Hand-rolled mutex-acquire … mutex-release (or open/close + dynamic-wind). A raise between acquire and release leaks the lock (deadlock) or the resource (fd/port leak) — the recurring deadlock/leak bug class (secmonlib ring.sls buffer-store! deadlock; secmon agent-server; drive write.ss; imagesite app.ss; browser passwords.ss; qt callback registry; sqlite-ffi leases).") + ("id" . "manual-mutex-without-with-lock") + ("kinds" "correctness" "concurrency" "security") + ("pattern" . "") ("severity" . "high") + ("tags" "mutex" "with-lock" "dynamic-wind" "deadlock" + "resource-leak" "with-resource") + ("title" + . + "Manual mutex-acquire/release instead of with-lock") + ("tools" "jerboa_howto")) + (("advice" + . + "cons onto a list and reverse once at the end, or use for/fold / append-map. For strings use an output port or string-join, not repeated string-append.") + ("avoid" + . + "Growing a list with (append acc (list x)) per iteration — each append copies the whole accumulator -> O(n^2). The single most common performance bug across the ecosystem (ssh channel/sftp, gitlab gl-get-all, webex store/gui, drive client/write, aws xml duplicate-child, imagesite, asm per-section, wormhole archive, semgrep expand-targets, browser keymap/minibuffer).") + ("id" . "append-in-loop-quadratic") ("kinds" "performance") + ("pattern" . "") ("severity" . "medium") + ("tags" "append" "quadratic" "for-fold" "append-map" + "performance" "cons-reverse") + ("title" . "(append acc (list x)) in a loop (O(n^2))") + ("tools" "jerboa_howto")) + (("advice" + . + "Make in-tree ignore/config OPT-IN (explicit flag); refuse security-critical keys (match-all exclude/allowlist, empty allowlist patterns, severity overrides, patterns.disabled, entropy:false, sub-floor size limits) from untrusted in-tree config; never honor in-tree baselines; emit a visible diagnostic listing every suppressed path so evasion is observable.") + ("avoid" + . + "A security scanner that auto-loads ignore/config/baseline from the scanned (hostile) tree. A malicious repo then disables its own detection: gitsafe .gitsafe.json exclude:[\"**\"] / allowlist.patterns:[\"\"] (string-contains \"\"->0 matches everything) / severity:\"critical\"; .gitsafeignore **; virus .jscanignore; semgrep .semgrepignore **. All produced SILENT detection-evasion.") + ("id" . "hostile-config-auto-trust") ("kinds" "security") + ("pattern" . "") ("severity" . "high") + ("tags" "scanner" "detection-evasion" "ignore-file" + "hostile-config" "fail-open" "baseline") + ("title" + . + "Trusting config/ignore/baseline from the scanned (hostile) tree") + ("tools" "jerboa_security_scan")) + (("advice" + . + "Use Encrypt-then-MAC (prefer ETM algorithms like hmac-sha2-256-etm@openssh.com); MAC the ciphertext including the encrypted length field; VERIFY the MAC BEFORE decrypting the body; make all pre-MAC failure messages indistinguishable.") + ("avoid" + . + "Computing the MAC over plaintext and decrypting the packet (including length/padding) BEFORE verifying the MAC (Encrypt-and-MAC). Distinguishable pre-MAC failures (invalid length / invalid padding) give a length/padding oracle. Seen in jerboa-ssh transport (only non-ETM hmac-sha2-256 offered).") + ("id" . "decrypt-before-verify-mac") ("kinds" "security") + ("pattern" . "") ("severity" . "high") + ("tags" "mac" "encrypt-then-mac" "etm" "padding-oracle" + "ssh" "verify-before-decrypt") + ("title" + . + "Encrypt-and-MAC / decrypting before verifying the MAC") + ("tools" "jerboa_security_scan")) + (("advice" + . + "Bind a monotonic per-direction seq into the AAD (direction || channel-id || seq); derive separate send/recv keys via HKDF info (direction domain separation); maintain a replay window/high-water mark and reject stale/replayed seq; fail closed. Apply identically in both peers of a shared contract.") + ("avoid" + . + "An authenticated transport that decrypts and executes frames with NO sequence number, EMPTY AAD, and ONE bidirectional key. A captured frame replays forever (jerboa-secmon/secmonlib captured 'acknowledge' purged buffered evidence without the PSK); empty AAD + shared key enables reflection and cross-context splicing.") + ("id" . "replay-no-sequence-aad") ("kinds" "security") + ("pattern" . "") ("severity" . "high") + ("tags" "replay" "aad" "sequence" "hkdf" "reflection" + "transport" "nonce") + ("title" + . + "AEAD transport without sequence/AAD/replay protection") ("tools" "jerboa_howto" "jerboa_security_scan"))) --- a/data/cookbooks.sexp +++ b/data/cookbooks.sexp @@ -6925,4 +6925,61 @@ "symlink" "tmp") ("title" . - "Generate unpredictable temp filenames with a cryptographic source"))) + "Generate unpredictable temp filenames with a cryptographic source")) + (("code" + . + ";; The native Rust Thompson-NFA engine is guaranteed-linear. Use it for\n;; search/replace/split/fold over any hostile pattern or subject.\n;; re-match? and re-fold-positions already use the native handle.\n;;\n;; For patterns needing backreferences/lookaround (which the linear engine\n;; rejects), fall back to pregexp explicitly and bound the match steps:\n(parameterize ((*pregexp-max-steps* 100000))\n (pregexp-match-positions pattern subject))\n;;\n;; A catastrophic pattern like \"(a+)+$\" against \"aaaaaaaaaaaaaaaaaaaaab\"\n;; must complete quickly (or raise a clean limit error) on the linear path.") ("id" . "linear-regex-default") ("imports" "(std regex)") + ("notes" + . + "The stdlib facade historically routed re-search/re-find-all/re-replace/re-split/re-fold through backtracking pregexp unconditionally -> ReDoS in every caller (gitsafe, semgrep, sinatra, webex, awk). Route these through the native engine when the pattern compiled natively (Rust rejects backrefs/lookaround, so those auto-fall-back) and the subject is ASCII (byte offsets == char offsets). Keep a backref regression test so the linear default doesn't break (a+)\\1.") + ("tags" "regex" "redos" "linear" "native" "pregexp" + "re-search") + ("title" + . + "Use the linear native regex engine by default (avoid ReDoS)")) + (("code" + . + ";; Before fetching an arbitrary/hostile-supplied URL:\n;; 1. Strip userinfo: http://user@evil/ -> host \"evil\" (part after last @)\n;; 2. Validate port is an exact integer in (0, 65536)\n;; 3. Resolve the host and block private/loopback/link-local/metadata:\n;; 127.0.0.0/8, ::1, 169.254.169.254 (cloud metadata),\n;; 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, localhost\n;; 4. Allowlist scheme to http/https; re-validate the destination on redirects.\n;; 5. Reject CRLF/NUL in caller-controlled header values; cap response body bytes.") ("id" . "ssrf-url-validation") + ("imports" "(std net request)") + ("notes" + . + "parse-url in (std net request) now strips userinfo and range-checks the port; an opt-in *http-ssrf-guard* blocks private ranges. For app-level fetch (e.g. jerboa-code tool/web.ss) resolve the numeric destination fail-closed and re-validate after any redirect. A redirect to a private IP must be refused. Also reject duplicate/conflicting Content-Length and CL+TE framing conflicts (response smuggling).") + ("tags" "ssrf" "url" "parse" "private-ip" "metadata" + "loopback" "redirect") + ("title" + . + "Validate URLs against SSRF (userinfo, port, private IPs)")) + (("code" + . + ";; Authenticated transport contract (jerboa-secmon/secmonlib):\n;; AAD = direction-byte(1) || channel-id(32) || seq(8 LE)\n;; channel-id = SHA256(handshake challenge nonce) ; binds to the session\n;; keys = HKDF(PSK, info=\"...:c2s\") / HKDF(PSK, info=\"...:s2c\")\n;; ; separate send/recv keys defeat reflection\n;; frame = seq(8 LE) || nonce(12) || ciphertext||tag\n;; replay = accept iff seq > high-water-mark; advance only after a\n;; successful AEAD open; reject stale/lower/replayed seq (fail closed)") ("id" . "replay-protection-aad-seq") ("imports") + ("notes" + . + "Empty AAD + one bidirectional key + no sequence let a captured frame (e.g. an 'acknowledge' that purges buffered evidence) replay forever and enabled reflection/cross-context splicing. Binding a monotonic per-direction seq into the AAD, deriving directional keys via HKDF info, and keeping a high-water mark closes replay + reflection. Apply identically in both peers of a shared contract.") + ("tags" "replay" "aead" "aad" "sequence" "hkdf" "reflection" + "transport") + ("title" + . + "Replay-protected AEAD transport (seq in AAD + HKDF direction keys)")) + (("code" + . + ";; WRONG: u8* expects a Scheme bytevector; a foreign-alloc pointer raises\n;; 'invalid foreign-procedure argument' (or mis-addresses under unsafe -> OOB).\n;; (define c-connect\n;; (foreign-procedure \"rustls_connect\" (u8* size_t) integer)) ;; BUG\n\n;; RIGHT: declare void* when passing foreign-alloc pointers (match read/write):\n(define c-connect\n (foreign-procedure \"rustls_connect\" (void* size_t) integer)) ;; OK\n\n;; Also: move the foreign-alloc INSIDE the try so a raise frees the buffer:\n(define (with-native-bytevector bv proc)\n (let ([p #f])\n (try (begin (set! p (foreign-alloc (bytevector-length bv)))\n (memcpy p bv) (proc p))\n (finally (when p (foreign-free p))))))") ("id" . "ffi-voidstar-for-foreign-alloc") + ("imports" "(chezscheme)") + ("notes" + . + "Seen in jerboa-gc: the collect-safety refactor switched buffer passing to foreign-alloc but left 5 tls-rustls connect decls and pcap c-pcap-next as u8*, so EVERY native TLS connect and pcap-next raised. u8* marshals a Scheme bytevector; void* passes the raw pointer. Also clamp any C-returned length against the buffer size before foreign-bv-sub (hostile/buggy native return -> OOB read).") + ("tags" "ffi" "u8*" "void*" "foreign-alloc" "type-mismatch" + "bounds") + ("title" + . + "Use void* (not u8*) for foreign-alloc pointers in FFI decls")) + (("code" + . + "// pgp-native/Cargo.toml — the FFI library MUST unwind so the guard works:\n// [profile.release]\n// panic = \"unwind\" ; NOT \"abort\" — abort makes catch_unwind a no-op\n//\n// src/lib.rs — wrap every extern \"C\" body:\n#[no_mangle]\npub extern \"C\" fn my_entry(/* ... */) -> i32 {\n guard(|| { /* ... body that may panic ... */ OK })\n}\nfn guard<F: FnOnce() -> i32 + std::panic::UnwindSafe>(f: F) -> i32 {\n match std::panic::catch_unwind(f) {\n Ok(code) => code,\n Err(_) => E_INTERNAL, // panic mapped to a clean error code\n }\n}") ("id" . "catch-panic-at-ffi-boundary") ("imports") + ("notes" + . + "With panic=\"abort\" the catch_unwind guard is a no-op — any dependency panic (e.g. the age crate on crafted input) aborts the whole CLI instead of returning an error; with panic=\"unwind\" but no catch_unwind, a panic unwinds across extern \"C\" = UB. You need BOTH: panic=\"unwind\" for the cdylib/staticlib AND catch_unwind(AssertUnwindSafe(..)) around each extern body. Add a config-guard test asserting the profile stays \"unwind\" (cargo test forces unwind for test targets, so a panic-through-guard test alone can't detect a profile revert). For a C ABI that bypasses a bounded-worker sandbox (jerboa-vision) also add a decode deadline/budget.") + ("tags" "rust" "ffi" "panic" "catch_unwind" "abort" "unwind" + "ub") + ("title" + . + "Catch Rust panics at the extern \"C\" boundary"))) --- a/data/security-rules.sexp +++ b/data/security-rules.sexp @@ -1150,4 +1150,80 @@ "get-bytevector-all reads an entire port into one bytevector. On a file or network stream whose size is large or attacker-controlled this is an unbounded-memory/OOM DoS (a 64 GiB advertised transfer OOMs the process). Found in jerboa-wormhole (transit read-blob, dir->archive).") ("pattern" . "get-bytevector-all") ("scope" . "scheme") - ("severity" . "medium"))) + ("severity" . "medium")) + (("id" . "backtracking-regex-on-hostile-input") + ("message" + . + "re-search/re-find-all/re-replace/re-split historically used backtracking pregexp; a hostile pattern or subject can trigger catastrophic backtracking (ReDoS). Use the linear native engine (re-fold-positions/native search path); fall back to pregexp only for patterns requiring backreferences/lookaround, and bound match steps.") + ("pattern" + . + "\\bre-search\\b|\\bre-find-all\\b|\\bre-replace\\b|\\bre-split\\b|pregexp-match") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "u8star-ffi-with-foreign-alloc") + ("message" + . + "A foreign-procedure declared with u8* expects a Scheme bytevector; passing a foreign-alloc pointer raises 'invalid foreign-procedure argument' (or mis-addresses under unsafe -> OOB read). Declare the param as void* when passing foreign-alloc pointers (match the read/write convention). Seen in jerboa-gc tls-rustls/pcap.") + ("pattern" . "u8\\*|foreign-alloc") + ("scope" . "ffi-boundary") + ("severity" . "high")) + (("id" . "unguarded-string-to-json-on-hostile") + ("message" + . + "string->json-object raises on malformed input; unguarded on a hostile body the raw parser error escapes typed error handling and can echo hostile bytes into UI/logs. Wrap in guard/try-result and map to a typed error; never surface raw parser text. Seen in webex/gitlab/drive API paths.") + ("pattern" . "string->json-object") + ("scope" . "scheme") + ("severity" . "medium")) + (("id" . "pagination-link-without-origin-check") + ("message" + . + "Following a server-supplied Link rel=\"next\" URL verbatim sends the Authorization bearer to whatever host the header names -> token exfiltration + cleartext downgrade + reading SSRF to loopback/cloud-metadata. Validate the next URL against the same canonical-HTTPS + exact-allowlisted-origin check used for the primary API before attaching credentials; stop pagination or strip auth off-origin. Seen in jerboa-webex.") + ("pattern" . "rel=.next|Link:|pagination|next-page") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "aead-random-nonce-under-static-key") + ("message" + . + "Using a random nonce under a never-rotated static key risks nonce reuse (catastrophic for GCM/ChaCha20-Poly1305) as message count grows toward the birthday bound. Use a deterministic counter/seq-derived nonce, XChaCha20-Poly1305 (24-byte nonce), or AES-GCM-SIV; rotate keys. Seen in jerboa-sshd telemetry, jerboa-signal logdb, jerboa-drive s3, wormhole transit.") + ("pattern" + . + "AEAD|GCM|chacha20-poly1305|random.*nonce|nonce.*random") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "panic-abort-defeats-catch-unwind-ffi-guard") + ("message" + . + "A Rust FFI crate with [profile.release] panic=\"abort\" makes any catch_unwind-based panic guard a no-op: a panic aborts the whole process instead of returning an error code (or unwinds across extern \"C\" = UB if unwind). Set panic=\"unwind\" for the cdylib/staticlib so catch_unwind catches panics at the boundary; wrap each extern \"C\" body in catch_unwind(AssertUnwindSafe(..)). Seen in jerboa-pgp (abort no-op) and jerboa-vision (missing catch_unwind).") + ("pattern" . "panic = .abort|catch_unwind") + ("scope" . "c-shim") + ("severity" . "high")) + (("id" . "scanner-auto-trusts-in-tree-config") + ("message" + . + "A security scanner that auto-loads ignore/config/baseline from the scanned (hostile) tree lets a malicious repo disable its own detection (exclude:[\"**\"], a bare ** ignore, an empty allowlist pattern that matches everything via string-contains->0). Make in-tree ignore/config opt-in; refuse security-critical keys from untrusted in-tree config; never honor in-tree baselines; emit a diagnostic of suppressed paths. Seen in gitsafe/virus/semgrep.") + ("pattern" + . + "\\.semgrepignore|\\.gitsafeignore|\\.gitsafe\\.json|\\.jscanignore|\\.virusignore") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "shell-interpolation-unquoted-into-sh") + ("message" + . + "Interpolating unquoted/unescaped user input into a command run via sh -c (open-process-ports/system) allows command injection (a single quote or $(...) breaks out). Pass user input as a separate argv element (no shell) via an execvp-style spawn, or shell-quote it; prefer a native library/binding. Seen in jerboa-coreutils expr (sed via sh -c), jerboa-code mentions, jerboa-emacs org-babel :dir.") + ("pattern" . "open-process-ports|\\bsystem\\b|sh -c") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "recursive-descent-follows-symlinks") + ("message" + . + "Recursive -R operations (chown/chgrp/chmod/rm) that descend using stat-based file-directory? follow symlinks, so a planted link to / re-owns/re-modes the target tree. Use lstat for descent, operate with fchownat/fchmodat(AT_SYMLINK_NOFOLLOW), and do not follow symlinks during recursion. Seen in jerboa-coreutils chown/chgrp/chmod -R.") + ("pattern" . "file-directory\\?|recursive|-R") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "archive-listing-line-based-drops-control-names") + ("message" + . + "Parsing zipinfo/bsdtar -tv output line-by-line silently drops archive members whose names contain CR/LF/NUL (the name splits across lines and the parser returns #f), letting a malicious archive hide a payload. Use NUL-delimited/native listing, or fail closed (flag the container as suspicious) on unparseable member names; reject names with NUL/control chars and insert -- before member args. Seen in jerboa-virus.") + ("pattern" . "zipinfo|bsdtar|tar -tv|unzip") + ("scope" . "scheme") + ("severity" . "high")))