From 138008d28d42060b4de0c8ccaad956098bf9b3ed Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Wed, 12 Aug 2026 16:16:34 +0200 Subject: [PATCH 1/3] protect: input-handling robustness pass across the engine and runtime A batch of defensive hardening so matching stays correct on inputs that don't take the obvious shape. No public API changes. - Host/IP classification (internal_host) now canonicalizes before deciding: every IPv4 spelling inet_aton accepts, and expanded / IPv4-mapped IPv6 forms, are recognized (previously a string/prefix compare); adds 100.64.0.0/10. - Origin/redirect comparisons resolve against the request origin and normalize default ports (off_origin handles protocol-relative / backslash locations; cross_origin distinguishes an absent header from a present opaque one; cors_reflected covers ACAO: null + credentials). - Scalar matchers fan out over the leaves of a structured (nested / array-of- object) value instead of stringifying it; bounded + iterative so a pathological value can't fail a rule open. - Regex safety: the ReDoS detector catches nested quantified subgroups, and a rejected pattern now warns (the rule is unenforced) instead of failing silent. - Request body handling: permissive content-type parsing (+json / text/plain / no content-type still populate post.); body inspection is no longer skipped on a declared Content-Length; `all` folds in the verbatim body. - Request normalization no longer deletes line-comment spans from the value it inspects (that hid payloads from parameter-scoped rules). - Response screening: exact content-type matching for live streams, a binary sniff so a textual octet-stream export is screened; a redactor whose rule decodes the body before matching now fails closed rather than serving a no-op mask; whitelist misconfig (no rule_id / unimplemented keys) warns. Adds tests/protect/security-hardening.test.ts plus updates to the normalizer / response-guards suites. 703 tests green. Co-Authored-By: Claude Opus 4.8 --- src/protect/engine/engine.js | 264 +++++++++++++++++++---- src/protect/engine/fetch.js | 64 ++++-- src/protect/engine/node.js | 28 +-- src/protect/engine/normalizer.js | 8 +- src/protect/engine/request.js | 6 + src/protect/runtime.js | 105 +++++++-- tests/protect/normalizer.test.ts | 14 +- tests/protect/response-guards.test.ts | 20 +- tests/protect/security-hardening.test.ts | 168 +++++++++++++++ 9 files changed, 561 insertions(+), 116 deletions(-) create mode 100644 tests/protect/security-hardening.test.ts diff --git a/src/protect/engine/engine.js b/src/protect/engine/engine.js index 0bd56f7..956a81c 100644 --- a/src/protect/engine/engine.js +++ b/src/protect/engine/engine.js @@ -4,13 +4,31 @@ import { normalizeRequest } from './normalizer.js'; // Catastrophic-backtracking shapes. Broad on purpose: a group whose inner content is quantified // (+, *, or {n,}) and is itself quantified — (a+)+, (\w+)+, (.*)*, ([a-z]+)*, (ab+)+ — or an // alternation under an outer quantifier — (a|a)*, (x|y)+. A rule matching one of these is skipped -// (safer than hanging the event loop). Earlier patterns only caught literal-letter groups and -// missed the far more common `\w`/`.`/char-class forms. +// (safer than hanging the event loop). The `NEST` variants allow ONE level of nested parentheses in +// the outer group so a quantified SUBGROUP is caught too (`((ab)+)+`, `((a|b)+)*`) — the earlier +// `[^)]*` forms stopped at the first inner `)` and missed those. `.test()` scans every start offset, +// so deeper nestings match at an inner window too. +const GRP = '(?:[^()]|\\([^()]*\\))*'; // group body allowing one level of nested parens const REDOS_PATTERNS = [ /\([^)]*[+*}][^)]*\)\s*[+*]/, - /\([^)]*\|[^)]*\)\s*[+*]/ + /\([^)]*\|[^)]*\)\s*[+*]/, + new RegExp('\\(' + GRP + '[+*}]' + GRP + '\\)\\s*[+*]'), // nested quantified subgroup + new RegExp('\\(' + GRP + '\\|' + GRP + '\\)\\s*[+*]') // nested alternation under a quantifier ]; +// Report once when a rule's regex is rejected (ReDoS-shaped or unparseable). Unlike an unknown match +// type, a rejected regex used to fail silently — so a delivered rule protected nothing and nobody knew. +const warnedRejectedPatterns = new Set(); +function warnRejectedPatternOnce(pattern) { + const key = String(pattern); + if (warnedRejectedPatterns.has(key)) return; + warnedRejectedPatterns.add(key); + console.warn( + `[patchstack] rule_v2 regex pattern rejected (unsafe or invalid) — condition treated as no-match. ` + + `The rule relying on it is NOT enforced: ${key}` + ); +} + export function safeRegExp(pattern) { if (!pattern) { return null; @@ -93,6 +111,44 @@ function arrayKeyValue(value, matchObj) { return false; } +// Match types that operate on the whole container (not per-leaf): `isset` (presence) and +// `array_in_array` / `array_key_value` (structural). Everything else is a scalar matcher that must +// fan out over the leaves of an object/array value. +const WHOLE_VALUE_MATCH_TYPES = new Set(['isset', 'array_in_array', 'array_key_value']); + +// Iteratively collect every scalar (non-object) leaf of a structured value. Iterative + bounded +// (depth and node caps) so a pathologically deep/large attacker payload STOPS at the bound rather +// than throwing a RangeError that the per-rule catch would swallow into a fail-open bypass. +function collectLeafValues(root, nodeCap = 20000, maxDepth = 1000) { + const out = []; + const stack = [[root, 0]]; + let visited = 0; + while (stack.length) { + const [node, depth] = stack.pop(); + if (node === null || node === undefined) continue; + if (typeof node !== 'object') { + out.push(node); + continue; + } + if (depth >= maxDepth || visited >= nodeCap) continue; + visited++; + if (Array.isArray(node)) { + for (let i = node.length - 1; i >= 0; i--) stack.push([node[i], depth + 1]); + } else { + for (const k of Object.keys(node)) stack.push([node[k], depth + 1]); + } + } + return out; +} + +// Emit a warning at most once per distinct key (keeps a persistent misconfiguration from spamming). +const warnedKeys = new Set(); +function warnOnce(key, message) { + if (warnedKeys.has(key)) return; + warnedKeys.add(key); + console.warn(message); +} + // Report an unknown/removed match type once, so a rule referencing it is not silently // unenforced (ADR: "unknown match type → skipped and logged, never silently passed"). const warnedMatchTypes = new Set(); @@ -108,45 +164,116 @@ function warnUnsupportedMatchType(type) { } // Internal / private / loopback / link-local / cloud-metadata host check, used by the -// `internal_host` match type for SSRF egress rules. Handles IPv4 (incl. IPv4-mapped IPv6), -// IPv6 loopback/link-local/unique-local, and localhost / *.local / GCP metadata names. +// `internal_host` match type for SSRF egress rules. It CANONICALIZES the host before classifying — +// a textual/prefix check is bypassable by alternate encodings (decimal/hex/octal IPv4, expanded or +// IPv4-mapped IPv6), which is a classic SSRF evasion. Handles localhost / *.local / GCP metadata +// names, every IPv4 spelling inet_aton accepts, and IPv6 loopback/link-local/unique-local/mapped. function isInternalHost(hostname) { if (!hostname) return false; - const host = String(hostname).toLowerCase().replace(/^\[|\]$/g, ''); + let host = String(hostname).toLowerCase().replace(/^\[|\]$/g, ''); + host = host.replace(/%[^\]]*$/, ''); // strip an IPv6 zone id (fe80::1%eth0) + host = host.replace(/\.$/, ''); // strip a single trailing dot (127.0.0.1.) if (host === 'localhost' || host.endsWith('.localhost') || host.endsWith('.local')) return true; if (host === 'metadata.google.internal') return true; - if (host === '::1' || host === '::') return true; - if (host.startsWith('fe80:')) return true; - // IPv6 unique-local (fc00::/7) — only when it is actually IPv6 (contains a colon), so ordinary - // hostnames that merely start with fc/fd (e.g. fcm.googleapis.com, fd-cdn.example.net) are not - // misclassified as internal and blocked. - if (host.includes(':') && (host.startsWith('fc') || host.startsWith('fd'))) return true; - - // Dotted IPv4 (incl. dotted IPv4-mapped `::ffff:127.0.0.1`, which ends in dotted form). - const v4 = host.match(/(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); - if (v4 && isPrivateV4(Number(v4[1]), Number(v4[2]))) return true; - - // Hex IPv4-mapped IPv6 (`::ffff:7f00:1` = 127.0.0.1) — Node's URL doesn't dotted-normalize this - // form, so classify it here too. - const mapped = host.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); - if (mapped) { - const g1 = parseInt(mapped[1], 16); - const g2 = parseInt(mapped[2], 16); - if (isPrivateV4((g1 >> 8) & 0xff, g1 & 0xff)) return true; + + // IPv6 (contains a colon): expand to 8 groups, then classify on the canonical form. + if (host.includes(':')) { + const g = expandIPv6(host); + if (!g) return false; + const allZeroHi = g[0] === 0 && g[1] === 0 && g[2] === 0 && g[3] === 0 && g[4] === 0; + if (allZeroHi && g[5] === 0 && g[6] === 0 && (g[7] === 0 || g[7] === 1)) return true; // ::, ::1 loopback + if ((g[0] & 0xffc0) === 0xfe80) return true; // link-local fe80::/10 + if ((g[0] & 0xfe00) === 0xfc00) return true; // unique-local fc00::/7 + if (allZeroHi && (g[5] === 0xffff || g[5] === 0)) { + // IPv4-mapped (::ffff:a.b.c.d) / IPv4-compatible (::a.b.c.d) — classify the embedded v4. + return isPrivateV4Int((((g[6] << 16) >>> 0) | g[7]) >>> 0); + } + return false; } + + // IPv4 in any inet_aton spelling (dotted quad, shorthand, decimal, hex, octal). + const v4 = parseIPv4ToInt(host); + if (v4 !== null) return isPrivateV4Int(v4); return false; } -// Private / loopback / link-local / this-host IPv4 test (first two octets are enough for our ranges). -function isPrivateV4(a, b) { +// Private / loopback / link-local / this-host / CGNAT test on a 32-bit IPv4 integer. +function isPrivateV4Int(n) { + const a = (n >>> 24) & 0xff; + const b = (n >>> 16) & 0xff; if (a === 127 || a === 10 || a === 0) return true; // loopback / private / this-host if (a === 169 && b === 254) return true; // link-local incl. 169.254.169.254 metadata if (a === 172 && b >= 16 && b <= 31) return true; // 172.16.0.0/12 if (a === 192 && b === 168) return true; // 192.168.0.0/16 + if (a === 100 && b >= 64 && b <= 127) return true; // 100.64.0.0/10 CGNAT (Alibaba metadata 100.100.100.200) return false; } +// inet_aton-style parse: 1–4 dot-separated parts, each decimal / 0x-hex / 0-octal; the final part +// fills the remaining low bytes. Returns a uint32, or null if the host isn't a numeric IPv4 form +// (so ordinary hostnames — which contain letters outside [0-9a-fx] or other chars — return null). +function parseIPv4ToInt(host) { + if (!/^[0-9a-fx]+(\.[0-9a-fx]+)*$/i.test(host)) return null; + const parts = host.split('.'); + if (parts.length > 4) return null; + const nums = []; + for (const p of parts) { + let n; + if (/^0x[0-9a-f]+$/i.test(p)) n = parseInt(p, 16); + else if (/^0[0-7]+$/.test(p)) n = parseInt(p, 8); + else if (/^[0-9]+$/.test(p)) n = parseInt(p, 10); + else return null; // e.g. a bare hex like "7f" (not inet_aton-valid without 0x) + if (!Number.isInteger(n) || n < 0) return null; + nums.push(n); + } + const last = nums.length - 1; + let value = 0; + for (let i = 0; i < last; i++) { + if (nums[i] > 0xff) return null; + value += nums[i] * 2 ** (8 * (3 - i)); + } + const maxLast = 2 ** (8 * (5 - nums.length)) - 1; // the final part fills the remaining low bytes + if (nums[last] > maxLast) return null; + value += nums[last]; + if (value < 0 || value > 0xffffffff) return null; + return value >>> 0; +} + +// Expand an IPv6 string (any `::` compression, optional embedded IPv4 tail) to 8 numeric groups, +// or null if it isn't valid IPv6. Lets the classifier compare the canonical form, not a string prefix. +function expandIPv6(host) { + if (!host.includes(':')) return null; + let s = host; + // Embedded IPv4 tail (::ffff:127.0.0.1) → convert the dotted part to two hex groups. + const lastColon = s.lastIndexOf(':'); + const tail = s.slice(lastColon + 1); + if (tail.includes('.')) { + const v4 = parseIPv4ToInt(tail); + if (v4 === null) return null; + s = s.slice(0, lastColon + 1) + ((v4 >>> 16) & 0xffff).toString(16) + ':' + (v4 & 0xffff).toString(16); + } + const halves = s.split('::'); + if (halves.length > 2) return null; + const head = halves[0] ? halves[0].split(':') : []; + const back = halves.length === 2 ? (halves[1] ? halves[1].split(':') : []) : null; + let groups; + if (back === null) { + groups = head; + } else { + const fill = 8 - head.length - back.length; + if (fill < 0) return null; + groups = [...head, ...Array(fill).fill('0'), ...back]; + } + if (groups.length !== 8) return null; + const out = []; + for (const g of groups) { + if (!/^[0-9a-f]{1,4}$/i.test(g)) return null; + out.push(parseInt(g, 16)); + } + return out; +} + // Route/method scope for a rule's optional `when: { method, path }`. Fail-open: if the scope can't // be evaluated, the rule still applies (never silently suppress a rule). function ruleAppliesTo(when, resolver) { @@ -179,16 +306,29 @@ function pathMatches(pattern, path) { return path === pattern; } -// CSRF primitive: does the request come from a different origin than its own Host? Lenient — a -// missing Origin/Referer (a non-browser client) is NOT treated as cross-origin. +// Drop a default port so `app.com:443` and `app.com` compare equal (a real proxy shape), without +// dropping a non-default port (so genuine cross-port stays distinguishable). +function normalizeDefaultPort(host) { + return String(host).toLowerCase().replace(/:(?:80|443)$/, ''); +} + +// CSRF primitive: does the request come from a different origin than its own Host? Lenient only when +// the Origin AND Referer are TRULY ABSENT (a non-browser client). A present-but-opaque `Origin: null` +// / empty / unparseable value is NOT same-origin — it's the sandboxed-iframe / opaque-origin signal a +// CSRF attacker supplies, so it is treated as cross-origin. function isCrossOrigin(resolver) { try { - const host = String(resolver.resolve('server.HTTP_HOST')[0] ?? '').toLowerCase(); + const host = normalizeDefaultPort(String(resolver.resolve('server.HTTP_HOST')[0] ?? '')); if (!host) return false; - const src = resolver.resolve('server.HTTP_ORIGIN')[0] ?? resolver.resolve('server.HTTP_REFERER')[0]; - if (!src) return false; - const srcHost = hostFromUrl(String(src)); - return srcHost !== null && srcHost !== host; + const originRaw = resolver.resolve('server.HTTP_ORIGIN')[0]; + const hasOrigin = originRaw !== undefined && originRaw !== null; + const src = hasOrigin ? originRaw : resolver.resolve('server.HTTP_REFERER')[0]; + if (src === undefined || src === null) return false; // both absent → lenient + const s = String(src).trim(); + if (hasOrigin && (s === '' || s.toLowerCase() === 'null')) return true; // present but opaque → cross-origin + const srcHost = hostFromUrl(s); + if (srcHost === null) return hasOrigin; // present-but-unparseable Origin → treat as cross-origin + return normalizeDefaultPort(srcHost) !== host; } catch { return false; } @@ -212,11 +352,21 @@ function isOffOriginRedirect(resolver) { if (status < 300 || status >= 400) return false; const location = resolver.resolve('response.header.location')[0]; if (!location) return false; - const target = hostFromUrl(String(location)); // null for a relative (same-origin) Location - if (target === null) return false; const host = String(resolver.resolve('server.HTTP_HOST')[0] ?? '').toLowerCase(); if (!host) return false; - return target !== host; + // Resolve the Location the way a browser would before comparing hosts: strip TAB/CR/LF and + // normalize backslashes to forward slashes (browsers do), then resolve against the request origin + // as a base. A relative `/path` resolves to our own host (not flagged); a protocol-relative + // `//evil.com` or backslash `/\evil.com` resolves off-origin (flagged) — the canonical + // open-redirect payloads that a base-less `new URL()` used to treat as "relative & safe". + const loc = String(location).replace(/[\t\r\n]/g, '').replace(/\\/g, '/'); + let target; + try { + target = new URL(loc, 'http://' + host).host.toLowerCase(); + } catch { + return false; // unresolvable even with a base → not a redirect we can judge + } + return normalizeDefaultPort(target) !== normalizeDefaultPort(host); } catch { return false; } @@ -234,6 +384,7 @@ function isReflectedCorsWithCredentials(resolver) { const acao = String(resolver.resolve('response.header.access-control-allow-origin')[0] ?? ''); if (!acao) return false; if (acao === '*') return true; // wildcard + credentials + if (acao.toLowerCase() === 'null') return true; // `null` + credentials is readable from a sandboxed iframe (Origin: null) const origin = String(resolver.resolve('server.HTTP_ORIGIN')[0] ?? ''); if (!origin) return false; return acao === origin; // ACAO echoes the caller's Origin → any origin is allowed @@ -253,7 +404,18 @@ export function matchValue(type, value, matchVal, matchObj) { return false; } - const strValue = typeof value === 'string' ? value : String(value); + // Guard the coercion: String() on a pathologically deep array/object can throw RangeError + // (stack overflow). Catching it here keeps a hostile nested value from failing the rule open. + let strValue; + if (typeof value === 'string') { + strValue = value; + } else { + try { + strValue = String(value); + } catch { + strValue = ''; + } + } switch (type) { case 'equals': @@ -280,6 +442,7 @@ export function matchValue(type, value, matchVal, matchObj) { case 'regex': { const regex = safeRegExp(matchVal); if (!regex) { + warnRejectedPatternOnce(matchVal); return false; } return regex.test(strValue); @@ -384,6 +547,19 @@ export class RuleEngine { this.#whitelists = whitelists; this.#whitelistKeys = whitelist_keys; this.#onError = onError; + // A whitelist with no `rule_id` suppresses EVERY rule when its (attacker-reachable) condition + // trips — almost never intended. And `whitelist_keys` is accepted but not implemented. Warn once + // for each so a misconfiguration that silently weakens the firewall is visible to the operator. + if (Array.isArray(whitelists) && whitelists.some((w) => w && Array.isArray(w.rule_v2) && !w.rule_id)) { + warnOnce( + 'whitelist-no-rule-id', + '[patchstack] a whitelist has no `rule_id` — it suppresses ALL rules when it matches. ' + + 'Scope each whitelist to a specific rule_id, and key it only on values an attacker cannot set.' + ); + } + if (whitelist_keys && typeof whitelist_keys === 'object' && Object.keys(whitelist_keys).length > 0) { + warnOnce('whitelist-keys-unimplemented', '[patchstack] `whitelist_keys` is not implemented and has no effect.'); + } } // A mitigation engine must never take down the app it protects: any error while @@ -527,10 +703,16 @@ export class RuleEngine { continue; } - if (typeof value === 'object' && value !== null && !Array.isArray(value)) { - for (const v of Object.values(value)) { - if (matchValue(match.type, v, match.value, match)) { - return true; + // A structured (object / array-of-object) value must be inspected at every leaf: a payload + // nested deeper than a scalar rule expects would otherwise stringify to "[object Object]" and + // evade the match, while the app still reads the live value. Whole-value match types + // (isset / array_in_array) see the container; scalar matchers fan out over all leaves. + if (typeof value === 'object' && value !== null) { + if (WHOLE_VALUE_MATCH_TYPES.has(match.type)) { + if (matchValue(match.type, value, match.value, match)) return true; + } else { + for (const leaf of collectLeafValues(value)) { + if (matchValue(match.type, leaf, match.value, match)) return true; } } continue; diff --git a/src/protect/engine/fetch.js b/src/protect/engine/fetch.js index 33b06d4..7daf353 100644 --- a/src/protect/engine/fetch.js +++ b/src/protect/engine/fetch.js @@ -42,25 +42,9 @@ export async function fromFetchRequest(request, options = {}) { let body = {}; let files; if (rawBody) { - if (contentType.includes('application/json')) { - try { - body = JSON.parse(rawBody); - } catch { - body = {}; - } - } else if (contentType.includes('application/x-www-form-urlencoded')) { - body = {}; - for (const [k, v] of new URLSearchParams(rawBody)) { - body[k] = k in body ? [].concat(body[k], v) : v; - } - } else if (contentType.includes('multipart/form-data')) { - const boundary = /boundary=("?)([^";]+)\1/i.exec(contentType)?.[2]; - if (boundary) { - const parsed = parseMultipart(rawBody, boundary); - body = parsed.body; - files = parsed.files; - } - } + const parsed = parseBody(rawBody, contentType); + body = parsed.body; + files = parsed.files; } const uri = url.pathname + url.search; @@ -90,9 +74,10 @@ export async function fromFetchRequest(request, options = {}) { // downstream handler keeps an intact body. (`max` is compared in bytes against Content-Length; the // prefix slice is by character, which can only over-scan a multibyte body — the safe direction.) async function readCappedText(request, max) { - const ceiling = Math.max(max, max * 4); - const declared = Number(request.headers?.get?.('content-length') || 0); - if (declared && declared > ceiling) return ''; + // Do NOT skip scanning based on a declared Content-Length: an attacker can declare a huge length + // (or none) to dodge inspection while sending a small exploit body. Always stream-scan the prefix + // up to `max` (buffering is bounded to `max`; the rest is drained but not retained). Anything past + // the cap is unscanned — the documented prefix-scan tradeoff — but the body is never skipped whole. let clone; try { clone = request.clone(); @@ -150,6 +135,41 @@ function concatChunks(chunks, total) { // Minimal multipart/form-data parser: enough to expose field names + values (so `post.` // and `raw` rules match uploads, e.g. a `__proto__` field name) and file metadata (filename via // `files.`). We only need the textual structure, not the binary file contents. +// Parse a request body into { body, files } for parameter-scoped rules. Content-type detection is +// deliberately permissive: many AI-built apps `JSON.parse(await req.text())` regardless of the +// declared type, so a JSON body arriving as `application/vnd.api+json`, `application/ld+json`, +// `text/plain`, `application/csp-report`, or with NO content-type must still populate post. +// (otherwise a field-scoped rule silently resolves to nothing). Unrecognized/binary bodies stay `{}` +// and are still matchable via `raw`. +export function parseBody(rawBody, contentType) { + const ct = String(contentType || '').toLowerCase(); + const isJson = ct.includes('application/json') || /\+json\b/.test(ct); + const isForm = ct.includes('application/x-www-form-urlencoded'); + const isMultipart = ct.includes('multipart/form-data'); + // "ambiguous" = a type an app commonly parses as JSON/form even though it isn't declared as such. + const isAmbiguous = ct === '' || ct.startsWith('text/plain') || ct.includes('csp-report') || ct.includes('/json'); + + if (isMultipart) { + const boundary = /boundary=("?)([^";]+)\1/i.exec(contentType)?.[2]; + if (boundary) return parseMultipart(rawBody, boundary); + return { body: {}, files: undefined }; + } + if (isForm) { + const body = {}; + for (const [k, v] of new URLSearchParams(rawBody)) body[k] = k in body ? [].concat(body[k], v) : v; + return { body, files: undefined }; + } + if (isJson || isAmbiguous) { + try { + const parsed = JSON.parse(rawBody); + if (parsed && typeof parsed === 'object') return { body: parsed, files: undefined }; + } catch { + /* not JSON — leave body empty; `raw`/`all` still see the verbatim text */ + } + } + return { body: {}, files: undefined }; +} + export function parseMultipart(rawBody, boundary) { const body = {}; const files = {}; diff --git a/src/protect/engine/node.js b/src/protect/engine/node.js index 7bfece7..790630b 100644 --- a/src/protect/engine/node.js +++ b/src/protect/engine/node.js @@ -7,7 +7,7 @@ // are already populated) and the Web-Fetch adapter (Workers/edge). Mount it FIRST, before // any body-parser — it consumes the stream and exposes the parsed body as `req.body`. import { RuleEngine } from './engine.js'; -import { parseMultipart } from './fetch.js'; +import { parseBody } from './fetch.js'; // Build the engine's request shape from a Node IncomingMessage + its raw body text. export function fromNodeRequest(req, rawBody = '') { @@ -45,27 +45,11 @@ export function fromNodeRequest(req, rawBody = '') { let body = {}; let files; if (rawBody) { - if (contentType.includes('application/json')) { - try { - body = JSON.parse(rawBody); - } catch { - body = {}; - } - } else if (contentType.includes('application/x-www-form-urlencoded')) { - body = {}; - for (const [k, v] of new URLSearchParams(rawBody)) { - body[k] = k in body ? [].concat(body[k], v) : v; - } - } else if (contentType.includes('multipart/form-data')) { - // Same parsing as the fetch adapter — expose field names/values via post. and file - // metadata via files., so field-scoped rules match uploads on a raw-Node server too. - const boundary = /boundary=("?)([^";]+)\1/i.exec(contentType)?.[2]; - if (boundary) { - const parsed = parseMultipart(rawBody, boundary); - body = parsed.body; - files = parsed.files; - } - } + // Same permissive content-type handling as the fetch adapter (+json / text/plain / no-CT bodies + // still populate post.; multipart exposes field + file metadata) on a raw-Node server too. + const parsed = parseBody(rawBody, contentType); + body = parsed.body; + files = parsed.files; } const uri = url.pathname + url.search; diff --git a/src/protect/engine/normalizer.js b/src/protect/engine/normalizer.js index f3f1b02..b757d2c 100644 --- a/src/protect/engine/normalizer.js +++ b/src/protect/engine/normalizer.js @@ -129,12 +129,16 @@ export function removeSqlComments(value) { return value; } + // Collapse inline block comments to a space (the anti-obfuscation goal). We must NOT strip the + // line-comment forms (`--…`, `#…`) to end-of-line: on the WAF inspection path that DELETES + // attacker-controlled spans from the value the engine sees while the app still processes the + // original — e.g. `#' } } }))).toBe(true); // depth 2 + expect(await blocks(p, jreq({ data: { a: '' } }))).toBe(true); // depth 1 control + }); + it('matches a payload inside an array of objects', async () => { + const p = await mk([{ id: 'i', rule_v2: [{ parameter: 'post.items', match: { type: 'contains', value: 'x' }] }))).toBe(true); + }); + it('matches within a sane depth and never crashes on a pathologically deep value', async () => { + const p = await mk([{ id: 'd', rule_v2: [{ parameter: 'post.q', match: { type: 'contains', value: 'evil' } }] }]); + // A realistically-nested payload is found. + let mid: any = 'evil'; + for (let i = 0; i < 100; i++) mid = [mid]; + expect(await blocks(p, jreq({ q: mid }))).toBe(true); + // A pathologically-deep value must not throw/hang (the RangeError fail-open) — the request just + // completes. (Nothing real nests this deep, and the app couldn't traverse it either.) + let deep: any = 'evil'; + for (let i = 0; i < 10000; i++) deep = [deep]; + await expect(blocks(p, jreq({ q: deep }))).resolves.toBeTypeOf('boolean'); + }); +}); + +describe('request: normalizer no longer deletes payload spans', () => { + it('matches a payload after a leading # (was deleted by comment-stripping)', async () => { + const p = await mk([{ id: 'x', rule_v2: [{ parameter: 'post.c', match: { type: 'contains', value: 'alert(1)' }))).toBe(true); + }); +}); + +describe('request: content-type parsing', () => { + const types = ['application/json', 'application/vnd.api+json', 'application/ld+json', 'text/plain', 'application/csp-report']; + it.each(types)('populates post.* for a JSON body sent as %s', async (ct) => { + const p = await mk([{ id: 'r', rule_v2: [{ parameter: 'post.role', match: { type: 'contains', value: 'admin' } }] }]); + expect(await blocks(p, jreq({ role: 'admin' }, ct))).toBe(true); + }); +}); + +describe('request: body cap is not skipped by a declared Content-Length', () => { + it('scans the prefix even when Content-Length is declared huge', async () => { + const p = await mk([{ id: 'pp', rule_v2: [{ parameter: 'raw', match: { type: 'contains', value: '__proto__' } }] }]); + const req = new Request('https://app.com/x', { + method: 'POST', + headers: { 'content-type': 'application/json', 'content-length': '9999999' }, + body: '{"__proto__":{"x":1}}', + }); + expect(await blocks(p, req)).toBe(true); + }); +}); + +describe('engine: ReDoS guard catches nested quantified subgroups', () => { + const { safeRegExp } = _testExports as any; + it('rejects ((ab)+)+ and deeper nestings', () => { + expect(safeRegExp('/((ab)+)+$/')).toBeNull(); + expect(safeRegExp('/(((ab)+)+)+$/')).toBeNull(); + expect(safeRegExp('/((a|b)+)+$/')).toBeNull(); + }); + it('keeps a genuinely safe pattern', () => { + expect(safeRegExp('/sk_live_[0-9A-Za-z]{16}/')).not.toBeNull(); + expect(safeRegExp('/AKIA[0-9A-Z]{16}/')).not.toBeNull(); + }); +}); + +describe('origin checks', () => { + it('cross_origin flags a present-but-opaque Origin: null (and normalizes default ports)', () => { + const eng = new RuleEngine({ firewall: [{ when: { method: ['POST'] }, rule_v2: [{ match: { type: 'cross_origin' } }] }] }); + const cx = (origin?: string) => eng.evaluate({ + method: 'POST', url: '/t', originalUrl: '/t', query: {}, body: {}, _rawBody: '', + headers: { host: 'app.com:443', ...(origin !== undefined ? { origin } : {}) }, + } as any).blocked; + expect(cx('null')).toBe(true); // opaque origin (sandboxed iframe) + expect(cx('https://evil.com')).toBe(true); // ordinary cross-origin + expect(cx('https://app.com')).toBe(false); // same host, default port elided → not cross-origin + expect(cx(undefined)).toBe(false); // truly absent → lenient + }); + + it('off_origin flags protocol-relative and backslash redirects', () => { + const eng = new RuleEngine({ firewall: [{ action: 'block', rule_v2: [{ match: { type: 'off_origin' } }] }] }); + const off = (loc: string) => eng.evaluate({ + method: 'GET', url: '/r', originalUrl: '/r', query: {}, body: {}, _rawBody: '', + headers: { host: 'app.com' }, _response: { status: 302, headers: { location: loc } }, + } as any).blocked; + expect(off('//evil.com/x')).toBe(true); + expect(off('/\\evil.com')).toBe(true); + expect(off('https://evil.com/x')).toBe(true); + expect(off('/safe-path')).toBe(false); // relative → same origin + }); + + it('cors_reflected flags ACAO: null + credentials', () => { + const eng = new RuleEngine({ firewall: [{ action: 'block', rule_v2: [{ match: { type: 'cors_reflected' } }] }] }); + const blocked = eng.evaluate({ + method: 'GET', url: '/x', originalUrl: '/x', query: {}, body: {}, _rawBody: '', headers: {}, + _response: { status: 200, headers: { 'access-control-allow-credentials': 'true', 'access-control-allow-origin': 'null' } }, + } as any).blocked; + expect(blocked).toBe(true); + }); +}); + +describe('response: content-type screening', () => { + const AWS = 'AKIA' + 'IOSFODNN7' + 'EXAMPLE'; + const served = async (contentType: string) => { + const p = await createProtection({ mode: 'block' }); + const resp = new Response(JSON.stringify({ apiKey: AWS }), { status: 200, headers: { 'content-type': contentType } }); + const out = await p.screenResponse(resp, new Request('https://app.com/x')); + return (await out.text()).includes(AWS); + }; + it('screens a body whose CT merely contains "event-stream" as a parameter', async () => { + expect(await served('application/json; profile="event-stream"')).toBe(false); + }); + it('still passes a real text/event-stream through unbuffered', async () => { + expect(await served('text/event-stream')).toBe(true); + }); + it('screens a textual octet-stream export', async () => { + expect(await served('application/octet-stream')).toBe(false); + }); +}); + +describe('response: mutation-carrying redactor fails closed', () => { + it('blocks (does not serve) when a redact rule decodes the body before matching', async () => { + const secret = 'sk_live_' + '0123456789abcdefXYZ'; + const b64 = Buffer.from(JSON.stringify({ token: secret })).toString('base64'); + const p = await createProtection({ + mode: 'block', + responseRules: [{ phase: 'response', action: 'redact', rule_v2: [{ parameter: 'response.body', mutations: ['base64_decode'], match: { type: 'contains', value: 'sk_live_' } }] }] as any, + }); + const out = await p.screenResponse(new Response(b64, { status: 200, headers: { 'content-type': 'text/plain' } }), new Request('https://app.com/x')); + expect(out.status).toBe(500); // withheld, not served with a no-op mask + expect((await out.text()).includes(secret)).toBe(false); + }); +}); From 178bfcf989bac74b3286e2c59b2e3d94ef4fd7d0 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Wed, 12 Aug 2026 16:23:04 +0200 Subject: [PATCH 2/3] protect: bound normalizeObject recursion; fix Node-version-fragile depth test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The request normalizer recursed into nested objects unbounded, so a pathologically deep value could overflow the stack before matching ran — the per-rule catch would swallow that into a fail-open. Cap the walk (values below the bound are left un-normalized, still matched, never crashing). The regression test built its deep value via a JSON string, which overflowed JSON.parse/stringify on Node 18/20/22 (but not 25) — a test artifact, not the engine. Rebuild it in memory and assert it still matches past the normalize cap (a fail-open crash would return blocked:false). Verified on Node 20 and 22. Co-Authored-By: Claude Opus 4.8 --- src/protect/engine/normalizer.js | 15 ++++++++++++--- tests/protect/security-hardening.test.ts | 19 +++++++++---------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/protect/engine/normalizer.js b/src/protect/engine/normalizer.js index b757d2c..692daa8 100644 --- a/src/protect/engine/normalizer.js +++ b/src/protect/engine/normalizer.js @@ -220,20 +220,29 @@ export function normalizeRequest(req, options = {}) { }; } -export function normalizeObject(value, options = {}) { +// Depth bound for the recursive walk: a pathologically deep object would otherwise overflow the +// stack, and the engine's per-rule catch would swallow that into a fail-open. Beyond the bound the +// sub-value is left un-normalized (still matched, just in its raw form) rather than crashing. +const MAX_NORMALIZE_DEPTH = 200; + +export function normalizeObject(value, options = {}, depth = 0) { if (typeof value === 'string') { return normalize(value, options); } + if (depth >= MAX_NORMALIZE_DEPTH) { + return value; + } + if (Array.isArray(value)) { - return value.map(item => normalizeObject(item, options)); + return value.map(item => normalizeObject(item, options, depth + 1)); } if (typeof value === 'object' && value !== null) { const result = {}; for (const [key, val] of Object.entries(value)) { - result[key] = normalizeObject(val, options); + result[key] = normalizeObject(val, options, depth + 1); } return result; diff --git a/tests/protect/security-hardening.test.ts b/tests/protect/security-hardening.test.ts index 9d42bc6..ee302d8 100644 --- a/tests/protect/security-hardening.test.ts +++ b/tests/protect/security-hardening.test.ts @@ -45,17 +45,16 @@ describe('request: structured-value evasion', () => { const p = await mk([{ id: 'i', rule_v2: [{ parameter: 'post.items', match: { type: 'contains', value: 'x' }] }))).toBe(true); }); - it('matches within a sane depth and never crashes on a pathologically deep value', async () => { - const p = await mk([{ id: 'd', rule_v2: [{ parameter: 'post.q', match: { type: 'contains', value: 'evil' } }] }]); - // A realistically-nested payload is found. - let mid: any = 'evil'; - for (let i = 0; i < 100; i++) mid = [mid]; - expect(await blocks(p, jreq({ q: mid }))).toBe(true); - // A pathologically-deep value must not throw/hang (the RangeError fail-open) — the request just - // completes. (Nothing real nests this deep, and the app couldn't traverse it either.) + it('matches past the normalize depth cap without a fail-open crash', async () => { + // Build the value in memory (a deep JSON string would overflow JSON.parse on older Node before it + // ever reached the engine). Depth 500 is beyond the normalizer's recursion cap (200), so if either + // the normalizer or the leaf walk still recursed unboundedly it would RangeError → the per-rule + // catch would fail the rule OPEN (blocked:false). A `true` here proves it walked through safely. let deep: any = 'evil'; - for (let i = 0; i < 10000; i++) deep = [deep]; - await expect(blocks(p, jreq({ q: deep }))).resolves.toBeTypeOf('boolean'); + for (let i = 0; i < 500; i++) deep = [deep]; + const eng = new RuleEngine({ firewall: [{ rule_v2: [{ parameter: 'post.q', match: { type: 'contains', value: 'evil' } }] }] }); + const res = eng.evaluate({ method: 'POST', url: '/', originalUrl: '/', query: {}, headers: {}, body: { q: deep }, _rawBody: '{}' } as any); + expect(res.blocked).toBe(true); }); }); From 96f7bf903abb0d12ac1c226f2e7edb654d1ceeee Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Wed, 12 Aug 2026 16:23:52 +0200 Subject: [PATCH 3/3] ci: also validate on Node 24 (the version we publish/release on) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI matrix tested 18/20/22, but publish.yml and release.yml build on Node 24 — so releases ran on a version CI never exercised. Add 24.x to close that gap. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53cf425..3cdccf6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,7 @@ jobs: - 18.x - 20.x - 22.x + - 24.x steps: - name: Checkout