diff --git a/.claude/skills/deep-code-review/templates/semgrep-taint/js.yml b/.claude/skills/deep-code-review/templates/semgrep-taint/js.yml index fcb78ffea..e1bf349c4 100644 --- a/.claude/skills/deep-code-review/templates/semgrep-taint/js.yml +++ b/.claude/skills/deep-code-review/templates/semgrep-taint/js.yml @@ -4,6 +4,9 @@ # Also date-parse-rollover (INFO: a date accepted only because Number.isFinite/isNaN of an inline Date.parse(x) or new Date(x) passed, with no .toISOString() round trip in the enclosing function; V8 reads 2026-02-30 as 2 March; test paths are skipped) and gcm-no-auth-tag-length (WARNING: createDecipheriv with an *-gcm cipher and no authTagLength option, so a truncated tag is accepted, a forgery risk only when the tag length is attacker-controlled). ponytail: any .toISOString() in the enclosing function counts as the round trip, component compares such as getUTCDate are not recognised, a numeric-timestamp new Date(ts) check can report, and a check on a variable bound first (const d = new Date(x); isNaN(d)) is not flagged because those hits were mostly noise; a cipher name that is not a string literal (or a const bound to one) and an options argument that is not an object literal are skipped. # Also body-cap-after-buffer (INFO: await req.formData()/json()/text()/arrayBuffer() on a receiver named like req or request (req, request, c.req, event.request) followed later in the same block by a .size/.length/.byteLength > cap on the result or on a .get(...) entry of it, or preceded only by a content-length > cap check; a comparison with the literal 0 is an emptiness check, not a cap, and a counted getReader() read passes; test, fixture and mock paths are skipped). ponytail: the receiver is matched by name (one ending in req or request), so a request bound to another name (r) or read through a call (req.clone().json()) is missed, only a > comparison with the result on the left is recognised, a result bound by destructuring or read inline (await req.formData()).get(...) is not tied to a later check, a ?. or ! on the result or on its .get(...) entry (body?.length, body!.length, form.get("f")!.size) is not matched, a deeper field path (body.items.length) is not the result, a read and its check in different blocks (the read inside a try, the check after it) are not tied, and a cap that lives in a middleware or framework option is invisible. # Also limiter-key-forwarded-header (INFO: an x-forwarded-for / x-real-ip read inside a function named like clientKey, limiterKey, rateKey, deriveKey or getIp; test, fixture and mock paths are skipped). ponytail: the function name is a heuristic, so a key built in an anonymous or differently named function (getClientIp, for one) is missed and any function with a matching name is reported. +# Also fetch-no-timeout (INFO: fetch(url) or fetch(url, {...}) with no signal key inside a Next route handler (export async function GET|POST|PUT|PATCH|DELETE), a pages API export default async function handler or an app.get|post|put|patch|delete|all("/path", ..., fn) callback; options with any signal value or a spread are skipped, as are test paths). ponytail: handlers are matched by shape, so a const-arrow export, a handler passed by name and a route path that is not a literal starting with / or * are not seen, and options held in a variable are read as unknown, not as missing a signal. +# Also auth-cookie-flags (WARNING: a cookie whose literal name has session, sess, sessid, sessionid, phpsessid, sid, token, auth, authtoken or jwt as a whole word (the name is split at punctuation and at lower-to-upper camelCase boundaries, and a name holding csrf or xsrf is skipped because double-submit cookies must stay readable by scripts), set positionally or as one options object by .cookie, .setCookie, cookies().set, (await cookies()).set, a variable bound by X = await cookies() or X = cookies() in an enclosing block (const jar = await cookies(); jar.set(...)) or .cookies.set on any receiver not named below, with httpOnly missing or the literal false, or with secure missing or the literal false; any other httpOnly or secure value passes; ctx.cookies.set, context.cookies.set, event.cookies.set, this.cookies.set and a bare cookies.set are reported only for a literal httpOnly false or secure false, because the cookies library behind Koa's ctx.cookies documents httpOnly as true by default and secure as true by default over HTTPS, and SvelteKit documents both as true by default (secure false in development); a receiver ending in req or request is a request-side cookie and is skipped, and a cookie cleared with an empty value, maxAge: 0 or expires: new Date(0) is skipped; test paths are skipped). ponytail: options must be an object literal (options held in a variable are not read and a spread is skipped), the browser cookieStore (a global, not bound from cookies()) is not seen, a bare cookies.set also covers other frameworks whose defaults were not checked, so their missing flags are not reported, and sameSite is not checked. +# Also cors-credentials-any-origin (WARNING: a cors(...) or register(cors, ...) call with credentials true and an origin of true, the regex /.*/ or a callback that always allows ((o, cb) => cb(null, true), cb(null, o) or cb(null, !!o), written as an arrow expression, an arrow block or a function); or a function that copies req.get|header("origin") or headers.get("origin"), directly or through a variable, into Access-Control-Allow-Origin while it also sets Access-Control-Allow-Credentials to "true" or true, unless an if on a .has, .includes or .test call encloses or precedes the set call in the same block (an allowlist check); origin "*" is not reported because the cors packages send a literal * and browsers refuse it on credentialed requests; req.headers.origin is left to the registry cors-misconfiguration rule; test paths are skipped). ponytail: options held in a variable and a credentials header given as an object are not seen, any such if counts as an allowlist check whatever it tests and a lookup written another way (indexOf, some) is not read, and a callback with other statements or a conditional is not matched. rules: - id: perun.js.sqli mode: taint @@ -818,3 +821,161 @@ rules: regex: (?i).*(?:(?:client|limiter|rate)key|derivekey|getip) paths: exclude: ["test", "tests", "__tests__", "spec", "fixtures", "mocks", "__mocks__", "e2e", "*.test.*", "*.spec.*"] +- id: perun.js.fetch-no-timeout + languages: [javascript, typescript] + severity: INFO + message: A server request handler calls fetch() with no signal option, so the call has no deadline of its own and a stalled upstream holds the request, its connection and its memory until a runtime or platform limit ends it. Pass a signal such as AbortSignal.timeout(ms) and handle the abort. + patterns: + - pattern-either: + - pattern: fetch($U) + - pattern: fetch($U, {...}) + - pattern-not: 'fetch($U, {..., signal: $S, ...})' + - pattern-not: 'fetch($U, {..., ...$X, ...})' + - pattern-either: + - patterns: + - pattern-inside: | + export async function $M(...) {...} + - metavariable-regex: + metavariable: $M + regex: ^(?:GET|POST|PUT|PATCH|DELETE)$ + - pattern-inside: | + export default async function handler(...) {...} + - patterns: + - pattern-either: + - pattern-inside: $A.$H("$P", ..., async (...) => {...}) + - pattern-inside: $A.$H("$P", ..., async function (...) {...}) + - pattern-inside: $A.$H("$P", ..., (...) => {...}) + - pattern-inside: $A.$H("$P", ..., function (...) {...}) + - metavariable-regex: + metavariable: $H + regex: ^(?:get|post|put|patch|delete|all)$ + - metavariable-regex: + metavariable: $P + regex: ^[/*] + paths: + exclude: ["test", "tests", "__tests__", "spec", "fixtures", "mocks", "__mocks__", "e2e", "*.test.*", "*.spec.*"] +- id: perun.js.auth-cookie-flags + languages: [javascript, typescript] + severity: WARNING + message: An authentication cookie (its name has session, sess, sessid, sessionid, phpsessid, sid, token, auth, authtoken or jwt as a whole word) is set without httpOnly, with httpOnly false, with no secure option or with secure false. Without httpOnly a script can read it, so one XSS steals the session, and without secure it also travels over plain HTTP. Set httpOnly true, secure true (or an environment check) and a sameSite value. + patterns: + - pattern-either: + - patterns: + - pattern-either: + - pattern: $C("$N", $V) + - pattern: $C("$N", $V, {...}) + - pattern: '$C({..., name: "$N", ...})' + - metavariable-regex: + metavariable: $C + regex: ^(?!(?:.*\.)?(?:req|request)\.cookies\.set$)(?:[\w.]+\.(?:cookie|setCookie)|(?:\(await )?cookies\(\)\)?\.set|(?:[\w.]+\.)?cookies\.set)$ + - patterns: + - pattern-either: + - pattern: $STORE.set("$N", $V) + - pattern: $STORE.set("$N", $V, {...}) + - pattern: '$STORE.set({..., name: "$N", ...})' + - pattern-either: + - pattern-inside: | + $STORE = await cookies(); + ... + - pattern-inside: | + $STORE = cookies(); + ... + - metavariable-regex: + metavariable: $N + regex: ^.*?(?:(? { $CB(null, true); }, ...})' + - pattern: '$F(..., {..., origin: ($O, $CB) => { $CB(null, $O); }, ...})' + - pattern: '$F(..., {..., origin: ($O, $CB) => { $CB(null, !!$O); }, ...})' + - patterns: + - pattern-either: + - pattern: $R.$M("$A", <... $REQ.$G("$O") ...>) + - patterns: + - pattern: $R.$M("$A", $V) + - pattern-inside: | + $V = <... $REQ.$G("$O") ...>; + ... + - metavariable-regex: + metavariable: $M + regex: ^(?:set|header|setHeader|append)$ + - metavariable-regex: + metavariable: $A + regex: (?i)^access-control-allow-origin$ + - metavariable-regex: + metavariable: $G + regex: ^(?:get|header)$ + - metavariable-regex: + metavariable: $O + regex: (?i)^origin$ + - pattern-either: + - pattern-inside: | + function $F(...) { + ... + <... $R2.$M2("$B", $T) ...> + ... + } + - pattern-inside: | + ($...P) => { + ... + <... $R2.$M2("$B", $T) ...> + ... + } + - metavariable-regex: + metavariable: $B + regex: (?i)^access-control-allow-credentials$ + - metavariable-regex: + metavariable: $T + regex: ^(?:true|["'`]true["'`])$ + - pattern-not-inside: + patterns: + - pattern: | + if (<... $X.$K(...) ...>) $S + ... + - metavariable-regex: + metavariable: $K + regex: ^(?:has|includes|test)$ + paths: + exclude: ["test", "tests", "__tests__", "spec", "fixtures", "mocks", "__mocks__", "e2e", "*.test.*", "*.spec.*"] diff --git a/changelog.d/semgrep-js-gap-rules.md b/changelog.d/semgrep-js-gap-rules.md new file mode 100644 index 000000000..2fb4e5501 --- /dev/null +++ b/changelog.d/semgrep-js-gap-rules.md @@ -0,0 +1,6 @@ +### Added +- Three bundled semgrep rules in `.claude/skills/deep-code-review/templates/semgrep-taint/js.yml` for defect classes the taint rules leave unflagged. Each carries the same test-path exclusions as `hardcoded-credential-compare`. +- `perun.js.fetch-no-timeout` (INFO): `fetch(url)` or `fetch(url, {...})` with no `signal` key inside a Next.js route handler (`export async function GET|POST|PUT|PATCH|DELETE`), a pages API `export default async function handler`, or an `app.get|post|put|patch|delete|all("/path", ..., fn)` callback. Any `signal:` value, an options spread, a fetch outside a handler and a `cache.get("key", fn)` callback stay clean. Ceiling: handlers are matched by shape, so a const-arrow export or a handler passed by name is not seen. +- `perun.js.auth-cookie-flags` (WARNING): a cookie whose literal name has session, sess, sessid, sessionid, phpsessid, sid, token, auth, authtoken or jwt as a whole word (split at punctuation and camelCase boundaries, so `authToken`, `connect.sid` and `next-auth.session-token` match and `authorized`, `tokenizer_mode` and `assessment` do not; names holding csrf or xsrf are skipped), set positionally or as one options object through `.cookie`, `.setCookie`, `cookies().set`, `(await cookies()).set`, a variable bound by `X = await cookies()` or `X = cookies()` in an enclosing block (`const jar = await cookies(); jar.set(...)`) or `.cookies.set` on any receiver not named below, with `httpOnly` missing or the literal `false`, or `secure` missing or the literal `false`. `ctx.cookies.set`, `context.cookies.set`, `event.cookies.set`, `this.cookies.set` and a bare `cookies.set` are reported only for a literal `httpOnly: false` or `secure: false`: the cookies library behind Koa's `ctx.cookies` documents `httpOnly` as true by default and `secure` as true by default over HTTPS, and SvelteKit documents both as true by default (`secure` false in development). Any other `httpOnly` or `secure` value passes, and a clear (an empty value, `maxAge: 0` or `expires: new Date(0)`) and a `req.cookies.set` or `request.cookies.set` call are skipped. Ceiling: options must be an object literal, and a bare `cookies.set` also covers other frameworks whose defaults were not checked, so their missing flags are not reported. +- `perun.js.cors-credentials-any-origin` (WARNING): `cors({...})` or `register(cors, {...})` with `credentials: true` and `origin: true`, `origin: /.*/` or a callback that always allows (`cb(null, true)`, `cb(null, origin)` or `cb(null, !!origin)`), or a function that copies `req.get|header("origin")` or `headers.get("origin")` (directly or through a variable) into `Access-Control-Allow-Origin` while it sets `Access-Control-Allow-Credentials` to `"true"` or `true`, unless an `if` on a `.has`, `.includes` or `.test` call encloses or precedes the set call. A literal `origin: "*"` is not reported, because the cors packages send a literal `*` and browsers refuse it on credentialed requests. The `req.headers.origin` handler shape is left to the default pack's express `cors-misconfiguration` rule, which already flags it. Ceiling: any such `if` counts as an allowlist check whatever it tests. +- Test: `scripts/test_semgrep_js_gaps.py`, registered in `scripts/ci-tests.d/semgrep-js-gaps.sh`, runs annotated fictional fixtures (a `ruleid` or `ok` comment marks the next line) through semgrep with the bundled rules only, plus copies under test paths that must stay clean. Offline checks of severity, exclusions and the key regexes run where semgrep is absent. diff --git a/docs/standards-index.md b/docs/standards-index.md index 324aa9788..ef588a059 100644 --- a/docs/standards-index.md +++ b/docs/standards-index.md @@ -1918,3 +1918,10 @@ Not fetched, by name only: the Homebrew formulae `gitleaks`, `trivy`, `osv-scann | ossf/scorecard — checks, Branch-Protection | https://raw.githubusercontent.com/ossf/scorecard/main/docs/checks.md | Fetched 2026-10-30: risk High; five settings (`DismissStaleReviews`, `EnforceAdmins`, `RequireLastPushApproval`, `RequiresStatusChecks`, `UpToDateBeforeMerge`) need an admin token, and with a non-admin token the check scores only the settings a non-admin can read; `EnforceAdmins` is false when any bypass actor is defined on any rule; rulesets are covered under "Repo Rules". Basis for "some Branch-Protection settings need an admin token" in `tool-hub.md`. | Not fetched, no URL: a community report that a token without admin can get a 404 from the branch-protection endpoint for a branch that is protected (seen in a search result, the page was not opened). The script therefore reports `Branch not protected` as a finding only when the repository object reports `permissions.admin` as true. `repo_settings_audit.py` was run offline against fixtures only; no live GitHub call was made when it was written, so the exact 403 and 404 bodies of each endpoint are the documented statuses, not observed ones. + +## Verified by direct fetch (2026-10-31) — cookie flag defaults (`templates/semgrep-taint/js.yml`, `perun.js.auth-cookie-flags`) + +| Source | URL | What it grounds | +|---|---|---| +| sveltejs/kit — @sveltejs/kit reference, Cookies | https://svelte.dev/docs/kit/@sveltejs-kit | Fetched 2026-10-31: the `Cookies` reference says "The `httpOnly` is `true` by default, as is `secure`, except during development, when it defaults to `false`." Basis for `event.cookies.set` being a receiver of `perun.js.auth-cookie-flags` that is reported only for a literal `httpOnly: false` or `secure: false`: a missing flag is the documented default there, not a defect. | +| pillarjs/cookies — README | https://github.com/pillarjs/cookies | Fetched 2026-10-31: `httpOnly` is "true by default"; `secure` is "false by default for HTTP, true by default for HTTPS". This is the cookies object behind Koa's `ctx.cookies`. Basis for `ctx.cookies.set`, `context.cookies.set`, `this.cookies.set` and a bare `cookies.set` being receivers of `perun.js.auth-cookie-flags` that are reported only for a literal `httpOnly: false` or `secure: false`. The defaults of other frameworks that expose a bare `cookies.set` were not checked, so their missing flags are not reported. | diff --git a/scripts/ci-tests.d/semgrep-js-gaps.sh b/scripts/ci-tests.d/semgrep-js-gaps.sh new file mode 100644 index 000000000..82575d3ef --- /dev/null +++ b/scripts/ci-tests.d/semgrep-js-gaps.sh @@ -0,0 +1,2 @@ +# Bundled semgrep rules perun.js.fetch-no-timeout, perun.js.auth-cookie-flags and perun.js.cors-credentials-any-origin: annotated ruleid/ok fictional fixtures (live when semgrep is installed, SKIP otherwise) plus offline checks of the rule text (severity, test-path exclusion, cookie-name, receiver and allowlist-guard regexes, signal and spread exemptions) that still run where semgrep is absent. Cookie receivers: a missing or literal-false httpOnly or secure is reported for res.cookie, reply.setCookie, cookies().set, a variable bound from cookies() and other .cookies.set receivers; for ctx, context, event, this and a bare cookies.set only a literal false is. +python3 scripts/test_semgrep_js_gaps.py diff --git a/scripts/test_semgrep_js_gaps.py b/scripts/test_semgrep_js_gaps.py new file mode 100644 index 000000000..5b0419589 --- /dev/null +++ b/scripts/test_semgrep_js_gaps.py @@ -0,0 +1,587 @@ +#!/usr/bin/env python3 +"""Tests for the bundled semgrep rules perun.js.fetch-no-timeout, perun.js.auth-cookie-flags and +perun.js.cors-credentials-any-origin (js.yml). + +Fixtures are annotated like semgrep's own tests: a `// ruleid: ` comment marks the NEXT line as a required finding, an +`// ok: ` comment marks the next line as one that must stay clean, and every unannotated line must stay clean too (so must a +copy of each fixture under a test path). All names and values are fictional. RULES= checks another copy of the rules (used to +show the cases fail without them). The live check needs semgrep and prints SKIP without it (CI installs none); the offline checks +always run. Plain asserts.""" +import json, os, re, shutil, subprocess, tempfile + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +RULES = os.environ.get("RULES") or os.path.join(ROOT, ".claude", "skills", "deep-code-review", "templates", "semgrep-taint") +FETCH, COOKIE, CORS = "perun.js.fetch-no-timeout", "perun.js.auth-cookie-flags", "perun.js.cors-credentials-any-origin" +IDS = (FETCH, COOKIE, CORS) + +FETCH_APP = r''' +export async function GET(request) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch("https://api.acme.example/v1/items"); + return Response.json(await r.json()); +} +export async function POST(request) { + const body = await request.json(); + // ruleid: perun.js.fetch-no-timeout + const r = await fetch("https://api.acme.example/v1/items", { method: "POST", body: JSON.stringify(body) }); + // ok: perun.js.fetch-no-timeout + const s = await fetch("https://api.acme.example/v1/audit", { method: "POST", signal: AbortSignal.timeout(5000) }); + return Response.json([await r.json(), s.status]); +} +export async function PUT(request) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(new URL("/v1/items", UPSTREAM), { method: "PUT", headers: { accept: "application/json" } }); + return Response.json(await r.json()); +} +export async function PATCH(request) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(`${UPSTREAM}/v1/items`, { method: "PATCH" }); + return Response.json(await r.json()); +} +export async function DELETE(request) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(`${UPSTREAM}/v1/items`, { method: "DELETE" }); + return Response.json(await r.json()); +} +app.get("/acme-api/items", async (req, res) => { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch("https://api.acme.example/v1/items", { headers: { accept: "application/json" } }); + res.json(await r.json()); +}); +router.post("/acme-api/items", function (req, res) { + // ruleid: perun.js.fetch-no-timeout + fetch(UPSTREAM + "/v1/items", { method: "POST", body: JSON.stringify(req.body) }).then((r) => res.sendStatus(r.status)); +}); +fastify.delete("/acme-api/items/:id", { schema }, async (request, reply) => { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(`${UPSTREAM}/v1/items/${request.params.id}`, { method: "DELETE" }); + return reply.code(r.status).send(); +}); +app.all("/acme-api/*", (req, res) => { + // ruleid: perun.js.fetch-no-timeout + fetch(UPSTREAM); + res.end(); +}); +app.patch("/acme-api/items/:id", async function (req, res) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(`${UPSTREAM}/v1/items/${req.params.id}`, { method: "PATCH" }); + res.json(await r.json()); +}); + +// Near misses: a signal of any kind, options that may carry one, and code that is not a request handler. +app.get("/acme-api/health", async (req, res) => { + // ok: perun.js.fetch-no-timeout + const r = await fetch("https://api.acme.example/v1/health", { signal: AbortSignal.timeout(5000) }); + res.sendStatus(r.status); +}); +app.post("/acme-api/sync", async (req, res) => { + const ctl = new AbortController(); + // ok: perun.js.fetch-no-timeout + const a = await fetch(UPSTREAM, { method: "POST", signal: ctl.signal, body: req.body }); + // ok: perun.js.fetch-no-timeout + const b = await fetch(UPSTREAM, { signal }); + res.json([a.status, b.status]); +}); +app.put("/acme-api/items/:id", async (req, res) => { + // ok: perun.js.fetch-no-timeout + const r = await fetch(UPSTREAM, { ...baseInit, method: "PUT" }); + res.json(await r.json()); +}); +async function warmCache() { + // ok: perun.js.fetch-no-timeout + return fetch("https://api.acme.example/v1/warm"); +} +// ok: perun.js.fetch-no-timeout +await fetch("https://api.acme.example/v1/ping"); +function ItemsWidget() { + useEffect(() => { + // ok: perun.js.fetch-no-timeout + fetch("/acme-api/items").then((r) => r.json()).then(setItems); + }, []); +} +cache.get("items", async () => { + // ok: perun.js.fetch-no-timeout + return fetch("https://api.acme.example/v1/items"); +}); +async function GET(request) { + // ok: perun.js.fetch-no-timeout + return fetch("https://api.acme.example/v1/items"); +} +''' + +FETCH_PAGES = r''' +export default async function handler(req, res) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(`https://api.acme.example/v1/items/${req.query.id}`); + // ruleid: perun.js.fetch-no-timeout + const s = await fetch("https://api.acme.example/v1/stats", { headers: { accept: "application/json" } }); + // ok: perun.js.fetch-no-timeout + const t = await fetch("https://api.acme.example/v1/slow", { signal: AbortSignal.timeout(2000) }); + res.status(200).json([await r.json(), await s.json(), await t.json()]); +} +''' + +# A typed Next.js handler and typed pages API handler: the TypeScript parser must reach them too. +FETCH_TS = r''' +export async function GET(request: NextRequest): Promise { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch(`https://api.acme.example/v1/items?q=${request.nextUrl.searchParams.get("q")}`); + return Response.json(await r.json()); +} +export default async function handler(req: NextApiRequest, res: NextApiResponse) { + // ruleid: perun.js.fetch-no-timeout + const r = await fetch("https://api.acme.example/v1/items", { method: "POST", body: JSON.stringify(req.body) }); + // ok: perun.js.fetch-no-timeout + const s = await fetch("https://api.acme.example/v1/items", { signal: AbortSignal.timeout(2000) }); + res.status(r.status).json(await s.json()); +} +''' + +COOKIE_JS = r''' +app.post("/acme-api/login", (req, res) => { + // ruleid: perun.js.auth-cookie-flags + res.cookie("session", token); + // ruleid: perun.js.auth-cookie-flags + res.cookie("auth_token", token, { httpOnly: true }); + // ruleid: perun.js.auth-cookie-flags + res.cookie("sid", sid, { secure: true }); + // ruleid: perun.js.auth-cookie-flags + res.cookie("jwt", jwt, { httpOnly: false, secure: true }); + // ruleid: perun.js.auth-cookie-flags + res.cookie("session", token, { httpOnly: true, secure: false }); + // ruleid: perun.js.auth-cookie-flags + res.cookie("session", token, { secure: false, httpOnly: true }); + res.end(); +}); +app.post("/acme-api/names", (req, res) => { + // ruleid: perun.js.auth-cookie-flags + res.cookie("connect.sid", sid); + // ruleid: perun.js.auth-cookie-flags + res.cookie("authToken", token); + // ruleid: perun.js.auth-cookie-flags + res.cookie("access-token", token); + // ruleid: perun.js.auth-cookie-flags + res.cookie("next-auth.session-token", token); + // ruleid: perun.js.auth-cookie-flags + res.cookie("__Host-session", token); + // ruleid: perun.js.auth-cookie-flags + res.cookie("PHPSESSID", sid); + // ruleid: perun.js.auth-cookie-flags + res.cookie("sessionid", sid); + // ruleid: perun.js.auth-cookie-flags + res.cookie("appSession", token); + // ruleid: perun.js.auth-cookie-flags + res.cookie("sessionKey", token); + res.end(); +}); +fastify.post("/acme-api/login", async (request, reply) => { + // ruleid: perun.js.auth-cookie-flags + reply.setCookie("session", token, { path: "/", maxAge: 3600 }); + // ruleid: perun.js.auth-cookie-flags + return reply.setCookie("Session-ID", token, { httpOnly: true }); +}); +export async function POST(request) { + // ruleid: perun.js.auth-cookie-flags + cookies().set("auth_token", token, { httpOnly: true }); + // ruleid: perun.js.auth-cookie-flags + cookies().set({ name: "session", value: token, httpOnly: true }); + // ruleid: perun.js.auth-cookie-flags + cookies().set({ secure: true, name: "session", value: token }); + // ruleid: perun.js.auth-cookie-flags + (await cookies()).set("jwt", token); + const response = NextResponse.json({}); + // ruleid: perun.js.auth-cookie-flags + response.cookies.set("session-token", token, { secure: true }); + // ruleid: perun.js.auth-cookie-flags + res.cookies.set("session", token); + // ruleid: perun.js.auth-cookie-flags + response.cookies.set("session", v); + return response; +} + +// Receivers whose libraries default httpOnly (and secure over HTTPS) on: only a literal false is reported there. +router.post("/acme-api/login", (ctx) => { + // ruleid: perun.js.auth-cookie-flags + ctx.cookies.set("session", v, { httpOnly: false }); + // ruleid: perun.js.auth-cookie-flags + ctx.cookies.set("session", v, { httpOnly: true, secure: false }); + // ruleid: perun.js.auth-cookie-flags + ctx.cookies.set({ name: "sid", value: v, secure: false }); + // ruleid: perun.js.auth-cookie-flags + context.cookies.set("auth_token", v, { secure: false }); + // ruleid: perun.js.auth-cookie-flags + this.cookies.set("jwt", v, { httpOnly: false }); + // ruleid: perun.js.auth-cookie-flags + event.cookies.set("session", v, { path: "/", secure: false }); + // ruleid: perun.js.auth-cookie-flags + cookies.set("sessionid", v, { path: "/", httpOnly: false }); + // ruleid: perun.js.auth-cookie-flags + cookies.set({ name: "session", value: v, httpOnly: false }); +}); + +// A cookies() store bound to a variable is a response store like cookies().set: a missing or literal-false flag is reported. +export async function PUT(request) { + const cookieStore = await cookies(); + // ruleid: perun.js.auth-cookie-flags + cookieStore.set("session", t); + // ruleid: perun.js.auth-cookie-flags + cookieStore.set({ name: "session", value: t, httpOnly: true }); + // ruleid: perun.js.auth-cookie-flags + cookieStore.set("sid", t, { httpOnly: true, secure: false }); +} +export function PATCH(request) { + let jar = cookies(); + // ruleid: perun.js.auth-cookie-flags + jar.set("authToken", t, { httpOnly: true }); +} + +// Near misses: both flags (either order, any value but a literal false), other cookie names, csrf cookies, clears, +// request-side cookies, options or names that are not literals, and (below) the library-default receivers. +app.get("/acme-api/prefs", (req, res) => { + // ok: perun.js.auth-cookie-flags + res.cookie("session", token, { httpOnly: true, secure: true, sameSite: "lax" }); + // ok: perun.js.auth-cookie-flags + res.cookie("sid", sid, { secure: process.env.NODE_ENV === "production", httpOnly: true }); + // ok: perun.js.auth-cookie-flags + reply.setCookie("auth", token, { httpOnly: true, secure }); + // ok: perun.js.auth-cookie-flags + res.cookie("session", token, { httpOnly, secure: true }); + // ok: perun.js.auth-cookie-flags + cookies().set({ name: "session", value: token, httpOnly: true, secure: true }); + // ok: perun.js.auth-cookie-flags + res.cookie("session", token, opts); + // ok: perun.js.auth-cookie-flags + res.cookie("session", token, { ...base, path: "/" }); + // ok: perun.js.auth-cookie-flags + res.cookie(name, token); + res.end(); +}); +app.get("/acme-api/names", (req, res) => { + // ok: perun.js.auth-cookie-flags + res.cookie("authorized", "yes"); + // ok: perun.js.auth-cookie-flags + res.cookie("tokenizer_mode", "fast"); + // ok: perun.js.auth-cookie-flags + res.cookie("assessment", "done"); + // ok: perun.js.auth-cookie-flags + res.cookie("csrftoken", token); + // ok: perun.js.auth-cookie-flags + res.cookie("XSRF-TOKEN", token); + // ok: perun.js.auth-cookie-flags + res.cookie("csrf_token", token); + // ok: perun.js.auth-cookie-flags + res.cookie("_csrf", token); + // ok: perun.js.auth-cookie-flags + res.cookie("theme", "dark"); + // ok: perun.js.auth-cookie-flags + res.cookie("sidebar_state", "open"); + res.end(); +}); +app.post("/acme-api/logout", (req, res) => { + // ok: perun.js.auth-cookie-flags + res.cookie("session", "", { path: "/" }); + // ok: perun.js.auth-cookie-flags + res.cookie("sid", sid, { maxAge: 0 }); + // ok: perun.js.auth-cookie-flags + cookies().set("session", token, { expires: new Date(0) }); + // ok: perun.js.auth-cookie-flags + cookies().set({ name: "session", value: "", path: "/" }); + res.end(); +}); +export function middleware(request) { + // ok: perun.js.auth-cookie-flags + request.cookies.set("session", token); + // ok: perun.js.auth-cookie-flags + req.cookies.set("auth_token", token); + // ok: perun.js.auth-cookie-flags + event.request.cookies.set("session", token); + // ok: perun.js.auth-cookie-flags + cookieStore.set("session", token, { maxAge: 60 }); + // ok: perun.js.auth-cookie-flags + Cookies.set("auth_token", token); + // ok: perun.js.auth-cookie-flags + cache.set("session", value); +} +// Receivers whose libraries default httpOnly (and secure over HTTPS) on: a missing flag passes. +router.get("/acme-api/prefs", (ctx) => { + // ok: perun.js.auth-cookie-flags + ctx.cookies.set("session", v); + // ok: perun.js.auth-cookie-flags + context.cookies.set("auth_token", v, { maxAge: 3600 }); + // ok: perun.js.auth-cookie-flags + this.cookies.set("jwt", v); + // ok: perun.js.auth-cookie-flags + event.cookies.set("session", v, { path: "/" }); + // ok: perun.js.auth-cookie-flags + cookies.set("sessionid", v, { path: "/" }); + // ok: perun.js.auth-cookie-flags + cookies.set({ name: "session", value: v }); + // ok: perun.js.auth-cookie-flags + ctx.cookies.set("session", v, { httpOnly: true, secure: true }); + // ok: perun.js.auth-cookie-flags + event.cookies.set("session", v, { path: "/", secure: process.env.NODE_ENV === "production" }); +}); +// A bound store with both flags passes, as do other names and clears; a .set on anything not bound from cookies() +// (the browser Cookie Store API, a cache, a parameter named like a store) passes even beside a bound store. +export async function DELETE(request) { + const cookieStore = await cookies(); + // ok: perun.js.auth-cookie-flags + cookieStore.set("session", t, { httpOnly: true, secure: true }); + // ok: perun.js.auth-cookie-flags + cookieStore.set("theme", "dark"); + // ok: perun.js.auth-cookie-flags + cookieStore.set("session", "", { maxAge: 0 }); +} +export async function OPTIONS(request) { + const jar = await cookies(); + let plain = cookies(); + // ok: perun.js.auth-cookie-flags + cookieStore.set("session", t); + // ok: perun.js.auth-cookie-flags + cache.set("session", t); + return Response.json({ id: jar.get("id"), n: plain.get("n") }); +} +export function HEAD(jar) { + // ok: perun.js.auth-cookie-flags + jar.set("session", t); +} +''' + +CORS_JS = r''' +// ruleid: perun.js.cors-credentials-any-origin +app.register(cors, { origin: true, credentials: true }); +// ruleid: perun.js.cors-credentials-any-origin +fastify.register(fastifyCors, { credentials: true, origin: true }); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ origin: /.*/, credentials: true })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ credentials: true, origin: (origin, cb) => cb(null, true) })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ credentials: true, origin: (origin, cb) => cb(null, origin) })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ credentials: true, origin: (origin, cb) => cb(null, !!origin) })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ credentials: true, origin: (origin, callback) => { callback(null, true); } })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ credentials: true, origin: (origin, cb) => { cb(null, origin); } })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ credentials: true, origin: (origin, cb) => { cb(null, !!origin); } })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ origin: function (origin, cb) { cb(null, true); }, credentials: true })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ origin: function (origin, cb) { cb(null, origin); }, credentials: true })); +// ruleid: perun.js.cors-credentials-any-origin +app.use(cors({ origin: function (origin, cb) { cb(null, !!origin); }, credentials: true })); +export async function GET(request) { + const headers = new Headers(); + // ruleid: perun.js.cors-credentials-any-origin + headers.set("Access-Control-Allow-Origin", request.headers.get("origin") ?? ""); + headers.set("Access-Control-Allow-Credentials", "true"); + return new Response("{}", { headers }); +} +app.get("/acme-api/me", (req, res) => { + // ruleid: perun.js.cors-credentials-any-origin + res.set("Access-Control-Allow-Origin", req.get("origin")); + res.set("Access-Control-Allow-Credentials", "true"); + res.json({}); +}); +app.use(function (req, res, next) { + const o = req.header("Origin"); + // ruleid: perun.js.cors-credentials-any-origin + res.header("Access-Control-Allow-Origin", o); + res.header("Access-Control-Allow-Credentials", "true"); + next(); +}); +app.get("/acme-api/boolean", (req, res) => { + // ruleid: perun.js.cors-credentials-any-origin + res.setHeader("Access-Control-Allow-Origin", req.header("Origin")); + res.setHeader("Access-Control-Allow-Credentials", true); + res.end(); +}); +app.options("/acme-api/*", (req, res) => { + const o = req.get("Origin"); + if (req.accepts("json")) res.status(204); + // ruleid: perun.js.cors-credentials-any-origin + res.set("Access-Control-Allow-Origin", o); + res.set("Access-Control-Allow-Credentials", "true"); + res.end(); +}); + +// Near misses: a named origin or allowlist, a literal "*" (browsers refuse it on credentialed requests), no credentials, +// credentials false, a header that is not the origin header, and a reflected origin checked against an allowlist first. +// ok: perun.js.cors-credentials-any-origin +app.register(fastifyCors, { credentials: true, origin: "*" }); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: "*", credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.register(cors, { origin: "https://app.acme.example", credentials: true }); +// ok: perun.js.cors-credentials-any-origin +app.register(cors, { origin: true }); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: "*" })); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: ALLOWED, credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: (o, cb) => cb(null, ALLOWED.has(o)), credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: (o, cb) => cb(null, other), credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: (o, cb) => { cb(null, ALLOWED.has(o)); }, credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: (o, cb) => { if (ALLOWED.has(o)) { cb(null, true); } else { cb(new Error("origin")); } }, credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.use(cors({ origin: function (o, cb) { if (ALLOWED.includes(o)) cb(null, true); else cb(new Error("origin")); }, credentials: true })); +// ok: perun.js.cors-credentials-any-origin +app.register(cors, { origin: true, credentials: false }); +// ok: perun.js.cors-credentials-any-origin +app.register(sensiblePlugin, { origin: true, credentials: true }); +export async function POST(request) { + const headers = new Headers(); + // ok: perun.js.cors-credentials-any-origin + headers.set("Access-Control-Allow-Origin", "https://app.acme.example"); + headers.set("Access-Control-Allow-Credentials", "true"); + return new Response("{}", { headers }); +} +export async function PUT(request) { + const headers = new Headers(); + // ok: perun.js.cors-credentials-any-origin + headers.set("Access-Control-Allow-Origin", request.headers.get("origin") ?? ""); + return new Response("{}", { headers }); +} +app.get("/acme-api/echo", (req, res) => { + // ok: perun.js.cors-credentials-any-origin + res.set("X-Acme-Echo", req.get("origin")); + res.set("Access-Control-Allow-Credentials", "true"); + res.json({}); +}); +app.get("/acme-api/off", (req, res) => { + // ok: perun.js.cors-credentials-any-origin + res.set("Access-Control-Allow-Origin", req.get("origin")); + res.set("Access-Control-Allow-Credentials", "false"); + res.json({}); +}); +app.get("/acme-api/has", (req, res) => { + const o = req.header("Origin"); + if (ALLOWED.has(o)) { + // ok: perun.js.cors-credentials-any-origin + res.header("Access-Control-Allow-Origin", o); + } + res.header("Access-Control-Allow-Credentials", "true"); + res.end(); +}); +app.get("/acme-api/includes", (req, res) => { + if (ALLOWED.includes(req.get("origin"))) { + // ok: perun.js.cors-credentials-any-origin + res.set("Access-Control-Allow-Origin", req.get("origin")); + } + res.set("Access-Control-Allow-Credentials", "true"); + res.end(); +}); +app.get("/acme-api/test", (req, res) => { + const o = req.header("Origin"); + // ok: perun.js.cors-credentials-any-origin + if (ALLOWED_RE.test(o)) res.header("Access-Control-Allow-Origin", o); + res.header("Access-Control-Allow-Credentials", "true"); + res.end(); +}); +app.use(function (req, res, next) { + const o = req.header("Origin"); + if (!ALLOWED.has(o)) return next(); + // ok: perun.js.cors-credentials-any-origin + res.header("Access-Control-Allow-Origin", o); + res.header("Access-Control-Allow-Credentials", "true"); + next(); +}); +app.get("/acme-api/else", (req, res) => { + const o = req.header("Origin"); + if (ALLOWED.has(o)) { + // ok: perun.js.cors-credentials-any-origin + res.header("Access-Control-Allow-Origin", o); + } else { + res.status(403); + } + res.header("Access-Control-Allow-Credentials", "true"); + res.end(); +}); +''' + +FILES = { + "app/api/acme-api/route.js": FETCH_APP, "app/api/acme-api/route.ts": FETCH_APP, "pages/api/acme-api/items.js": FETCH_PAGES, + "pages/api/acme-api/typed.ts": FETCH_TS, "app/auth/login.js": COOKIE_JS, "app/auth/login.ts": COOKIE_JS, + "app/web/cors.js": CORS_JS, "app/web/cors.ts": CORS_JS, +} +# Test, fixture and mock paths are excluded from every one of these rules: nothing in these copies may be reported. +SKIPPED = {"tests/route.test.js": FETCH_APP, "app/login.spec.ts": COOKIE_JS, "app/fixtures/cors.js": CORS_JS} +ANN = re.compile(r"^\s*(?://|#)\s*(ruleid|ok):\s*(\S+)\s*$") + + +def annotated(body, kind): + """(line, rule id) of the line each `// : ` comment points at (the next one); the id must be one of ours.""" + out = {(i + 1, m.group(2)) for i, l in enumerate(body.split("\n"), 1) if (m := ANN.match(l)) and m.group(1) == kind} + assert {r for _, r in out} <= set(IDS), out + return out + + +# Offline: the spec's floor of 3 findings and 3 near misses per rule, counted over distinct fixtures (the .js and .ts copies +# of one fixture are one set of cases). +for rid in IDS: + for kind in ("ruleid", "ok"): + n = sum(1 for body in {*FILES.values()} for _, r in annotated(body, kind) if r == rid) + assert n >= 3, (rid, kind, n) +assert all(annotated(b, "ok") and annotated(b, "ruleid") for b in FILES.values()) +if shutil.which("semgrep"): + with tempfile.TemporaryDirectory() as d: + for p, body in {**FILES, **SKIPPED}.items(): + os.makedirs(os.path.dirname(os.path.join(d, p)), exist_ok=True) + open(os.path.join(d, p), "w").write(body) + env = dict(os.environ, HOME=d, SEMGREP_USER_HOME=d) + run = subprocess.run(["semgrep", "--config", RULES, "--metrics=off", "--disable-version-check", "--json", "--quiet", "--", *FILES, *SKIPPED], + capture_output=True, text=True, env=env, cwd=d) + r = json.loads(run.stdout) + assert not r["errors"], r["errors"] + got = set() # semgrep prefixes a rule id with each local config's directory: keep the perun.. tail + for x in r["results"]: + rid = "perun." + x["check_id"].rsplit("perun.", 1)[-1] + if rid in IDS: + got.add((x["path"], x["start"]["line"], rid)) + want = {(p, ln, rid) for p, body in FILES.items() for ln, rid in annotated(body, "ruleid")} + assert got == want, ("unexpected", sorted(got - want), "missed", sorted(want - got)) +else: + print("SKIP: semgrep not installed; the live fixture check did not run, the offline checks below do") + +# Offline: ids, severities, shared test-path exclusions, and the regexes (semgrep anchors metavariable-regex at the start, +# as Python's re.match does) that carry the precision. CI installs no semgrep, so each guard the live fixtures prove is +# pinned here too: dropping one fails this file even when the live part is skipped. +JS = open(os.path.join(RULES, "js.yml")).read() +block = lambda rid: JS.split(f"- id: {rid}\n")[1].split("\n- id:")[0] +excl = lambda b: (re.search(r"paths:\n\s+exclude: (\[.*\])", b) or [0, 0])[1] +for rid, sev in ((FETCH, "INFO"), (COOKIE, "WARNING"), (CORS, "WARNING")): + b = block(rid) + assert f" severity: {sev}\n" in b and JS.count(f"- id: {rid}\n") == 1, rid + assert excl(b) and excl(b) == excl(block("perun.js.hardcoded-credential-compare")), f"{rid} skips the credential rules' test paths" +rx = lambda b, mv: re.findall(rf"metavariable: \${mv}\n\s+regex: (.*)", b) +fb, kb, cb = block(FETCH), block(COOKIE), block(CORS) +assert fb.count("pattern-not: ") == 2 and fb.count("signal: $S") == 1 and "...$X" in fb, "a signal option or a spread exempts the call" +assert "regex: ^[/*]" in fb and "GET|POST|PUT|PATCH|DELETE" in fb and "get|post|put|patch|delete|all" in fb, "route path and verb guards" +cn, cs = rx(kb, "N") +cc, cx = rx(kb, "C") +assert all(re.match(cn, s) for s in ("session", "connect.sid", "authToken", "auth_token", "access-token", "next-auth.session-token", "jwt", + "__Host-session", "Session-ID", "PHPSESSID", "sessionId", "appSession", "sessionKey", "sess", "sessid", "sessionid", "authtoken", "phpsessid", + "sid", "token", "auth")), cn +assert not any(re.match(cn, s) for s in ("authorized", "tokenizer_mode", "assessment", "csrftoken", "theme", "sidebar_state", "side", "ssid", "tokens")), cn +assert all(re.match(cs, s) for s in ("session", "auth_token")) and not any(re.match(cs, s) for s in ("XSRF-TOKEN", "csrf_token", "_csrf", "x-csrf-token")), cs +assert all(re.match(cc, s) for s in ("res.cookie", "reply.setCookie", "cookies().set", "(await cookies()).set", "response.cookies.set", "res.cookies.set")), cc +assert not any(re.match(cc, s) for s in ("Cookies.set", "map.set", "cache.set", "cookieJar.set", "res.send", "cookieStore.set", "request.cookies.set", + "req.cookies.set", "event.request.cookies.set")), cc +assert all(re.match(cx, s) for s in ("cookies.set", "ctx.cookies.set", "context.cookies.set", "event.cookies.set", "this.cookies.set")), cx +assert not any(re.match(cx, s) for s in ("res.cookies.set", "response.cookies.set", "this.ctx.cookies.set", "Cookies.set", "ctx.cookies.setx", "xctx.cookies.set", + "cookies().set", "(await cookies()).set", "res.cookie", "reply.setCookie", "event.request.cookies.set")), cx +assert kb.count("pattern-not: '$C(..., {..., httpOnly: false, ...})'") == kb.count("pattern-not: '$C(..., {..., secure: false, ...})'") == 1, "only a literal false counts there" +assert rx(kb, "H") == rx(kb, "S") == ["^(?!false$)"], "a literal false httpOnly or secure is reported, any other value passes" +assert kb.count("$STORE.set(") == 3 and kb.count("$STORE = await cookies();") == kb.count("$STORE = cookies();") == 1, "a cookies() store bound to a variable is read through its binding" +assert "headers.origin" not in cb, "the registry cors-misconfiguration rule owns req.headers.origin" +assert 'origin: "*"' not in cb, "browsers refuse a literal * origin on credentialed requests, so it is not this defect" +ct, ck = rx(cb, "T")[0], rx(cb, "K")[0] +assert all(re.match(ct, s) for s in ("true", '"true"', "'true'", "`true`")) and not any(re.match(ct, s) for s in ("false", '"false"', "1", "trueish")), ct +assert all(re.match(ck, s) for s in ("has", "includes", "test")) and not any(re.match(ck, s) for s in ("some", "indexOf", "match")), ck +print("ok") diff --git a/scripts/test_semgrep_taint.py b/scripts/test_semgrep_taint.py index c49d8ea82..d06c3a479 100755 --- a/scripts/test_semgrep_taint.py +++ b/scripts/test_semgrep_taint.py @@ -63,11 +63,11 @@ def n2(h): fs.readFile(path.join(ROOT, f), () => {}); }); app.get("/n1", (req, res) => { m.get(req.query.k); document.body.innerHTML = "a"; }); -app.get("/n2", (req, res) => { const u = req.query.u; if (!H.has(u)) { log(); } fetch(u); }); +app.get("/n2", (req, res) => { const u = req.query.u; if (!H.has(u)) { log(); } fetch(u, { signal }); }); app.get("/ok3", async (req, res) => { const u = new URL(req.query.url); if (!H.has(u.hostname)) { return res.status(400).end(); } - await fetch(u); + await fetch(u, { signal }); }); ''') env = dict(os.environ, HOME=d, SEMGREP_USER_HOME=d)