diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eed75d..eddfb83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,51 @@ All notable changes follow [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.2.0] — 2026-06-21 + +Security hardening of the argument sanitizer and a new verify-then-attest +helper. All three packages move to 0.2.0 in lockstep. + +### Security + +- **ReDoS guard on `regex` argument rules.** `evalArgRule` now refuses to run a + regex pattern whose structure is prone to catastrophic backtracking (nested + unbounded quantifiers like `(a+)+`, `(.*)+`, `(.*a){15}`). The pattern comes + from the signed manifest, but the *value* is attacker-controlled — a careless + author pattern previously let a single crafted argument freeze the spawn hot + path for tens of seconds. Detection is a linear-time scan of the pattern + string (the detector itself cannot ReDoS) and fails closed. New optional + `maxLength` field on `regex` rules (default 4096) caps the input length as a + second layer. Exposed as `looksCatastrophic(pattern)`. +- **Path-traversal guard on `prefix` argument rules.** A bare `startsWith` + check was bypassable: `/safe/../../etc/passwd` satisfies `prefix: "/safe/"` + yet escapes the directory. `prefix` rules now reject `..` path components + (including the URL-encoded `%2e%2e` form, POSIX and Windows separators) by + default. New optional `denyTraversal` field (default `true`) opts out. + The secure default is enforced at evaluation time, so it also protects + manifests constructed in-memory and signed directly (where the Zod default + has not materialised). Exposed as `containsTraversal(value)`. +- **Control-character gap closed.** `shellSafeString` now also blocks VT + (U+000B), FF (U+000C) and NEL (U+0085), completing the newline/whitespace + separator default-deny set alongside the existing LF, CR, U+2028 and U+2029. + These can act as token separators or line breaks under some shells, parsers + and NFKC normalisation. CVE-2025-69256 replay fixtures extended accordingly. + +### Added + +- `attestSpawnVerified(signed, request, options?)` — verifies the manifest + signature *then* attests the spawn in a single fail-safe call. Closes the + footgun where an unverified or tampered manifest is handed to `attestSpawn` + (which trusts that the caller already ran `verifyManifestStrict` at startup). + +### Notes + +- The `prefix` rule's `denyTraversal: true` default is a behaviour change: a + pre-existing manifest that relied on `..` passing a prefix rule will now + reject it. This is intentional (a `..` in a path-prefix-guarded argument is + almost always an attack); set `denyTraversal: false` to restore the old + behaviour. + ## [0.1.1] — 2026-04-28 ### Added @@ -45,4 +90,5 @@ Initial release. - Per-entry validation of trust-file contents with `Object.create(null)` containers. +[0.2.0]: https://github.com/studiomeyer-io/mcp-server-attestation/releases/tag/v0.2.0 [0.1.0]: https://github.com/studiomeyer-io/mcp-server-attestation/releases/tag/v0.1.0 diff --git a/README.md b/README.md index 05acb73..47356b3 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,15 @@ attestSpawnStrict(signed as SignedManifest, { command, args }); That is the entire integration. Two function calls, no SaaS, no daemon. +If you would rather re-verify the signature on every spawn (defense-in-depth +against an unverified or swapped manifest reaching the gate), use the single +fail-safe call instead — it verifies *then* attests: + +```ts +import { attestSpawnVerified, type SignedManifest } from "mcp-server-attestation"; +attestSpawnVerified(signed as SignedManifest, { command, args }); +``` + ## Tools (reference server `mcp-attest-demo`) | # | Name | readOnlyHint | destructiveHint | @@ -105,7 +114,10 @@ The library is transport-agnostic. The reference server is stdio-only. - **Trust-on-First-Use** is the default. The first time you verify a server, its public key is pinned to `~/.mcp-attest/trust.json` (override: `MCP_ATTEST_TRUST_FILE`). Subsequent verifications reject any new key for the same server name with `TRUST_PIN_MISMATCH`. This catches the Cursor-style malicious-update vector. - **No bundled trusted-keys list.** This package does not act as a gatekeeper. If you want stronger assurance, opt into `--sigstore` to cross-reference the public-key fingerprint against the Sigstore Rekor transparency log. -- **Default-deny argument sanitizer.** `shellSafeString` blocks every ASCII shell metacharacter, NUL, CR, LF, zero-width characters, BOM, RTL/LTR overrides, and Trojan-Source isolates. Allowlist behaviour requires the explicit `regex` / `enum` / `prefix` / `literal` rule kinds. +- **Default-deny argument sanitizer.** `shellSafeString` blocks every ASCII shell metacharacter, NUL, CR, LF, VT, FF, NEL, zero-width characters, BOM, RTL/LTR overrides, Trojan-Source isolates, and fullwidth-Latin confusables. Allowlist behaviour requires the explicit `regex` / `enum` / `prefix` / `literal` rule kinds. +- **ReDoS-safe `regex` rules.** A `regex` rule's pattern is signed by the author, but the argument value is attacker-controlled. The sanitizer statically detects backtracking-prone patterns (nested unbounded quantifiers like `(a+)+`) and refuses to run them, so a single crafted argument cannot freeze the spawn hot path. `regex` rules also carry a `maxLength` input cap (default 4096). +- **Traversal-safe `prefix` rules.** `prefix` rules reject `..` path components by default (`denyTraversal: true`, including the `%2e%2e` encoded form), so `/safe/../../etc/passwd` is blocked even though it satisfies `prefix: "/safe/"`. +- **Verify-then-attest.** `attestSpawnVerified` checks the manifest signature before attesting the spawn in one fail-safe call — use it instead of `attestSpawnStrict` unless you have measured the per-spawn Ed25519 verify out of a genuinely hot loop. - **Canonical JSON** is the signed surface. Re-serialisation cannot change the signed bytes. What this package does NOT do (out of scope): diff --git a/docs/MANIFEST-FORMAT.md b/docs/MANIFEST-FORMAT.md index 7f22f9b..169a109 100644 --- a/docs/MANIFEST-FORMAT.md +++ b/docs/MANIFEST-FORMAT.md @@ -47,12 +47,23 @@ Tool name must match `/^[a-zA-Z_][a-zA-Z0-9_-]*$/`. Up to 64 args per tool, 256 | `kind` | Fields | Semantics | | ------ | ------ | --------- | -| `regex` | `pattern`, `flags` | Argument must match the regex. | +| `regex` | `pattern`, `flags`, `maxLength` | Argument must match the regex. Patterns prone to catastrophic backtracking (nested unbounded quantifiers) are **refused, not run** — see ReDoS note below. `maxLength` (default 4096) caps the input length handed to the engine. | | `enum` | `values[]` | Argument must equal one of the listed values. | | `length` | `min`, `max` | Argument string length within `[min, max]`. | -| `prefix` | `prefix`, `maxSuffixLength` | Argument starts with `prefix`, suffix length under `maxSuffixLength`. | +| `prefix` | `prefix`, `maxSuffixLength`, `denyTraversal` | Argument starts with `prefix`, suffix length under `maxSuffixLength`. `denyTraversal` (default `true`) rejects any `..` path component, including the URL-encoded `%2e%2e` form — so `/safe/../../etc/passwd` is blocked even though it satisfies `prefix: "/safe/"`. Set `false` only when `..` is legitimately part of the value. | | `literal` | `value` | Argument exactly equals `value`. | -| `shellSafeString` | `maxLength` | No shell metacharacters, no NUL/CR/LF, no zero-width, no BOM, no bidi-override. | +| `shellSafeString` | `maxLength` | No shell metacharacters, no NUL/CR/LF/VT/FF/NEL, no zero-width, no BOM, no bidi-override, no fullwidth-Latin confusables. | + +### ReDoS note + +A `regex` rule's `pattern` is part of the signed manifest, but the argument +*value* is attacker-controlled. A careless author pattern such as `(a+)+$` lets +a single crafted argument freeze the spawn hot path for tens of seconds. The +sanitizer statically detects nested-unbounded-quantifier patterns (`(a+)+`, +`(a*)*`, `(.*)+`, `(.*a){15}`, `((ab)*)*`, ...) with a linear-time scan and +**refuses to evaluate them**, failing closed. Flat patterns — `^[a-z]+$`, +`^\d{1,10}$`, `(foo|bar)`, `(ab){2,5}`, semver — are unaffected. If your +pattern is rejected, rewrite it without a nested quantifier. `shellSafeString` is the right default for any argument that ends up in `child_process.spawn` argv — the sanitizer is implemented in `packages/lib/src/spawn.ts`. @@ -62,13 +73,13 @@ Tool name must match `/^[a-zA-Z_][a-zA-Z0-9_-]*$/`. Up to 64 args per tool, 256 { "command": "/usr/bin/cat", "args": [ - { "name": "file", "kind": "prefix", "required": true, "prefix": "/safe/", "maxSuffixLength": 256 } + { "name": "file", "kind": "prefix", "required": true, "prefix": "/safe/", "maxSuffixLength": 256, "denyTraversal": true } ], "maxTotalArgLength": 1024 } ``` -`maxTotalArgLength` is a hard cap on the sum of argv string lengths and is the primary defense against buffer-stuffing attacks. +`maxTotalArgLength` is a hard cap on the sum of argv string lengths and is the primary defense against buffer-stuffing attacks. `denyTraversal` defaults to `true`; the example shows it explicitly for clarity. ## Trust file (`~/.mcp-attest/trust.json`) diff --git a/docs/THREAT-MODEL.md b/docs/THREAT-MODEL.md index d1e48d1..5a20757 100644 --- a/docs/THREAT-MODEL.md +++ b/docs/THREAT-MODEL.md @@ -5,10 +5,13 @@ | Threat | Vector | Mitigation in this package | | --- | --- | --- | | Marketplace poisoning (OX Security April 2026) | Registry accepts a malicious server impersonating a known one | TOFU pin in `~/.mcp-attest/trust.json` rejects key changes for an already-trusted server name. Optional Sigstore cross-reference. | -| CVE-2025-69256 (Serverless Framework MCP RCE) | Tool argument fed to `child_process.exec()` without escaping | `shellSafeString` argument rule blocks every ASCII shell metacharacter, NUL, CR, LF, zero-width, BOM, bidi-override. Argv length capped. | +| CVE-2025-69256 (Serverless Framework MCP RCE) | Tool argument fed to `child_process.exec()` without escaping | `shellSafeString` argument rule blocks every ASCII shell metacharacter, NUL, CR, LF, VT, FF, NEL, zero-width, BOM, bidi-override, fullwidth-Latin confusables. Argv length capped. | | CVE-2025-61591 (Cursor MCP RCE) | Malicious server spawns unrelated commands | Spawn-rule whitelist: `attestSpawn` rejects any command not in `manifest.spawnRules[]`. | | Manifest tampering | Attacker changes a tool description or arg-rule after signing | Ed25519 signature over canonical JSON of the entire manifest. Any byte flip fails verify. | | Fingerprint spoofing | Manifest claims one fingerprint, signed by another key | Both `signManifest` (refuses to sign) and `verifyManifest` (rejects) check that claimed fingerprint matches the embedded public key. | +| Argument-rule ReDoS (denial of service) | A `regex` arg rule with a backtracking-prone pattern; attacker sends a crafted value that hangs the spawn hot path | `evalArgRule` statically detects nested-unbounded-quantifier patterns (linear scan, cannot itself ReDoS) and refuses to run them. `regex` rules also carry a `maxLength` input cap (default 4096). | +| Path traversal past a `prefix` guard | `/safe/../../etc/passwd` satisfies a `prefix: "/safe/"` rule yet escapes the directory | `prefix` rules reject `..` path components by default (`denyTraversal: true`), including the URL-encoded `%2e%2e` form. | +| Unverified manifest reaching the spawn gate | Caller forgets to `verifyManifestStrict` at startup and hands an unverified/tampered manifest to `attestSpawn` | `attestSpawnVerified` verifies the signature before attesting in one fail-safe call. | ## What this package does NOT mitigate diff --git a/package-lock.json b/package-lock.json index 869d9e6..b85bec6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "mcp-server-attestation-monorepo", - "version": "0.1.0", + "version": "0.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "mcp-server-attestation-monorepo", - "version": "0.1.0", + "version": "0.2.0", "license": "MIT", "workspaces": [ "packages/lib", @@ -2436,10 +2436,10 @@ }, "packages/cli": { "name": "mcp-attest-cli", - "version": "0.1.0", + "version": "0.2.0", "license": "MIT", "dependencies": { - "mcp-server-attestation": "0.1.0" + "mcp-server-attestation": "0.2.0" }, "bin": { "mcp-attest": "dist/bin.js" @@ -2450,7 +2450,7 @@ }, "packages/demo-server": { "name": "mcp-attest-demo", - "version": "0.1.1", + "version": "0.2.0", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", @@ -2464,9 +2464,30 @@ "node": ">=20.0.0" } }, + "packages/demo-server/node_modules/mcp-server-attestation": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/mcp-server-attestation/-/mcp-server-attestation-0.1.0.tgz", + "integrity": "sha512-yS1EYMGl8SPP+C1J8gdTWJqT15RPOZtw/o5HuAqijWFGxw7fXmkvodDPhfaUlXeMy8kK3EGFN/6TXmEbL7NuWQ==", + "license": "MIT", + "dependencies": { + "zod": "^3.23.0" + }, + "engines": { + "node": ">=20.0.0" + } + }, + "packages/demo-server/node_modules/mcp-server-attestation/node_modules/zod": { + "version": "3.25.76", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", + "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + }, "packages/lib": { "name": "mcp-server-attestation", - "version": "0.1.0", + "version": "0.2.0", "license": "MIT", "dependencies": { "zod": "^4.4.3" diff --git a/package.json b/package.json index b6a95a5..0d95a47 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mcp-server-attestation-monorepo", - "version": "0.1.0", + "version": "0.2.0", "private": true, "description": "Tool-Manifest-Attestation for Model Context Protocol. Layer-2 mitigation for marketplace-poisoning (OX Security April 2026), CVE-2025-69256 (Serverless MCP RCE), CVE-2025-61591 (Cursor MCP RCE). Ed25519 signed manifests, runtime spawn-attestation, default-deny argument sanitizer.", "type": "module", diff --git a/packages/cli/package.json b/packages/cli/package.json index 829dd4d..4793cfb 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "mcp-attest-cli", - "version": "0.1.0", + "version": "0.2.0", "description": "CLI for MCP tool-manifest attestation: keygen, sign, verify, inspect, fingerprint, check-pin.", "type": "module", "main": "./dist/bin.js", @@ -30,6 +30,6 @@ "url": "https://github.com/studiomeyer-io/mcp-server-attestation/issues" }, "dependencies": { - "mcp-server-attestation": "0.1.0" + "mcp-server-attestation": "0.2.0" } } diff --git a/packages/demo-server/package.json b/packages/demo-server/package.json index 52fa0e0..632018d 100644 --- a/packages/demo-server/package.json +++ b/packages/demo-server/package.json @@ -1,6 +1,6 @@ { "name": "mcp-attest-demo", - "version": "0.1.1", + "version": "0.2.0", "mcpName": "io.studiomeyer/server-attestation", "description": "Reference MCP server demonstrating mcp-server-attestation library — 5 tools for keygen, sign, verify, template-generation, and spawn-inspection. stdio transport, MCP spec 2025-06-18.", "type": "module", @@ -9,14 +9,26 @@ "bin": { "mcp-attest-demo": "./dist/server.js" }, - "files": ["dist", "README.md", "LICENSE"], + "files": [ + "dist", + "README.md", + "LICENSE" + ], "scripts": { "build": "tsc -b && chmod +x dist/server.js", "typecheck": "tsc --noEmit", "start": "node dist/server.js" }, - "keywords": ["mcp", "model-context-protocol", "attestation", "demo", "reference-server"], - "engines": { "node": ">=20.0.0" }, + "keywords": [ + "mcp", + "model-context-protocol", + "attestation", + "demo", + "reference-server" + ], + "engines": { + "node": ">=20.0.0" + }, "license": "MIT", "author": "Matthias Meyer (StudioMeyer)", "publishConfig": { @@ -37,7 +49,9 @@ "zod": "^4.4.3" }, "mcp": { - "supportedSpecVersions": ["2025-06-18"], + "supportedSpecVersions": [ + "2025-06-18" + ], "transport": "stdio" } } diff --git a/packages/lib/package.json b/packages/lib/package.json index 1f615d3..0f08c73 100644 --- a/packages/lib/package.json +++ b/packages/lib/package.json @@ -1,6 +1,6 @@ { "name": "mcp-server-attestation", - "version": "0.1.0", + "version": "0.2.0", "description": "Library for Ed25519-signed MCP tool manifests, runtime spawn-attestation, default-deny argument sanitizer. Layer-2 mitigation for marketplace-poisoning, CVE-2025-69256, CVE-2025-61591.", "type": "module", "main": "./dist/index.js", diff --git a/packages/lib/src/index.ts b/packages/lib/src/index.ts index 5f86f79..675244b 100644 --- a/packages/lib/src/index.ts +++ b/packages/lib/src/index.ts @@ -32,8 +32,11 @@ export type { VerificationResult } from "./verify.js"; export { attestSpawn, attestSpawnStrict, + attestSpawnVerified, sanitizeArgs, evalArgRule, + looksCatastrophic, + containsTraversal, } from "./spawn.js"; export type { SpawnRequest, SanitizationResult, AttestationResult } from "./spawn.js"; diff --git a/packages/lib/src/manifest.ts b/packages/lib/src/manifest.ts index e4d558c..6a915dd 100644 --- a/packages/lib/src/manifest.ts +++ b/packages/lib/src/manifest.ts @@ -25,6 +25,17 @@ export const ArgRuleSchema = z.discriminatedUnion("kind", [ kind: z.literal("regex"), pattern: z.string().min(1).max(2048), flags: z.string().max(8).optional(), + /** + * Hard cap on the length of the value handed to the regex engine. + * + * ReDoS defense-in-depth: catastrophic backtracking is super-linear in the + * input length, so bounding the attacker-controlled value bounds the worst + * case. `evalArgRule` rejects over-long values *before* calling `.test()`. + * The primary ReDoS guard is the static pattern check (see `spawn.ts` + * `looksCatastrophic`), which refuses to run a backtracking-prone pattern + * at all; this cap is the second layer. Default 4096. + */ + maxLength: z.number().int().positive().max(65536).default(4096), }), baseArgRule.extend({ kind: z.literal("enum"), @@ -39,6 +50,19 @@ export const ArgRuleSchema = z.discriminatedUnion("kind", [ kind: z.literal("prefix"), prefix: z.string().min(1).max(512), maxSuffixLength: z.number().int().nonnegative().max(65536).default(2048), + /** + * Reject `..` path segments anywhere in the value. The `prefix` rule's + * documented purpose is path-guarding (e.g. `prefix: "/safe/"`), but a bare + * `startsWith` check is bypassable: `/safe/../../etc/passwd` satisfies the + * prefix yet escapes the directory. With `denyTraversal` (default `true`) + * any value containing a `..` path component — `../`, `..\`, a leading or + * trailing `..`, or URL-encoded `%2e%2e` — is rejected. + * + * Default-on is the safe choice for a security package: a `..` inside a + * path-prefix-guarded argument is almost always an attack. Set to `false` + * to opt out when `..` is legitimately part of the value (rare). + */ + denyTraversal: z.boolean().default(true), }), baseArgRule.extend({ kind: z.literal("literal"), diff --git a/packages/lib/src/spawn.ts b/packages/lib/src/spawn.ts index 94f42a7..1dffc85 100644 --- a/packages/lib/src/spawn.ts +++ b/packages/lib/src/spawn.ts @@ -6,6 +6,7 @@ import { type ToolDecl, } from "./manifest.js"; import { AttestationError } from "./errors.js"; +import { verifyManifest, type VerifyOptions } from "./verify.js"; /** * Default-deny shell-metacharacter set blocked by `shellSafeString` rules. @@ -33,6 +34,13 @@ const FORBIDDEN_CODEPOINTS: ReadonlySet = (() => { 0x00, // null 0x0a, // LF 0x0d, // CR + // Other C0 control whitespace some shells/parsers treat as token + // separators or line breaks. LF/CR are handled above; VT, FF and NEL + // complete the newline/whitespace-separator default-deny so an attacker + // cannot smuggle a line break past a `shellSafeString` rule. + 0x0b, // VT (vertical tab) + 0x0c, // FF (form feed) + 0x85, // NEL (Unicode next-line; acts as a newline under NFKC / many terminals) // Zero-width / formatting 0x200b, 0x200c, 0x200d, 0x200e, 0x200f, // Bidi overrides @@ -112,6 +120,29 @@ export function evalArgRule(value: unknown, rule: ArgRule): string[] { } switch (rule.kind) { case "regex": { + // ReDoS guard (primary): refuse to run a pattern whose structure is + // prone to catastrophic backtracking. The pattern comes from the signed + // manifest, but the *value* is attacker-controlled — a careless author + // pattern like `(a+)+$` lets a crafted value hang the spawn hot path for + // tens of seconds. We never execute such a pattern; we fail closed. + if (looksCatastrophic(rule.pattern)) { + reasons.push( + `arg "${rule.name}" rule uses a regex prone to catastrophic backtracking (nested unbounded quantifiers); refusing to evaluate. Rewrite the pattern without nested quantifiers like (a+)+`, + ); + return reasons; + } + // ReDoS guard (defense-in-depth): bound the attacker-controlled input + // length before handing it to the engine. Fall back to 4096 when the + // value is absent so the cap also applies to manifests built in-memory + // (where the Zod default has not materialised) — same principle as the + // prefix denyTraversal default. + const regexMaxLength = rule.maxLength ?? 4096; + if (value.length > regexMaxLength) { + reasons.push( + `arg "${rule.name}" length ${value.length} exceeds regex maxLength ${regexMaxLength}`, + ); + return reasons; + } let re: RegExp; try { re = new RegExp(rule.pattern, rule.flags); @@ -144,6 +175,18 @@ export function evalArgRule(value: unknown, rule: ArgRule): string[] { if (suffix.length > rule.maxSuffixLength) { reasons.push(`arg "${rule.name}" suffix length ${suffix.length} exceeds max ${rule.maxSuffixLength}`); } + // Path-traversal guard: a bare prefix check is bypassable via `..`. + // `/safe/../../etc/passwd` satisfies `prefix: "/safe/"` but escapes the + // directory. Reject `..` path components anywhere in the value unless + // the rule explicitly opts out with `denyTraversal: false`. + // + // We test `!== false` rather than truthiness so the secure behaviour + // also holds for manifests constructed in-memory and signed directly + // (where the Zod `.default(true)` has not materialised) — the safe + // default must apply at the point of enforcement, not only after parse. + if (rule.denyTraversal !== false && containsTraversal(value)) { + reasons.push(`arg "${rule.name}" contains a path-traversal segment (".."); refused by prefix rule`); + } } break; } @@ -169,6 +212,122 @@ export function evalArgRule(value: unknown, rule: ArgRule): string[] { return reasons; } +/** + * Reject values containing a `..` path-traversal segment. Conservative: only + * a genuine `..` *component* trips this — bounded by a path separator, the + * string boundary, or its URL-encoded form `%2e%2e`. A literal `..` embedded + * in a longer token (e.g. `file..name`) is NOT a traversal component and is + * left alone, so legitimate values are not over-blocked. + * + * Handles both POSIX (`/`) and Windows (`\\`) separators and percent-encoding. + */ +export function containsTraversal(value: string): boolean { + const lowered = value.toLowerCase(); + // Percent-encoded `..` (covers %2e%2e and mixed `.%2e` / `%2e.`). + const decodedDots = lowered.replace(/%2e/g, "."); + // A `..` component is one bounded on both sides by a separator or boundary. + // [/\\] or start/end of string. We test the decoded form so encoded + // traversal is caught too. + return /(^|[/\\])\.\.([/\\]|$)/.test(decodedDots); +} + +/** + * Static, O(n) detector for regex patterns prone to catastrophic backtracking. + * + * It scans the pattern string (never executes it, so the detector itself + * cannot ReDoS) and flags the structural cause of exponential/polynomial + * blowup: a group that contains an unbounded quantifier (`*`, `+`, `{n,}`) and + * is itself amplified by another quantifier — `(a+)+`, `(a*)*`, `(.*)+`, + * `(.*a){15}`, `((ab)*)*`, etc. + * + * Conservative by design: it flags nested-quantifier constructs (the dangerous + * ones) and leaves flat patterns — `^[a-z]+$`, `^\d{1,10}$`, `(foo|bar)`, + * `(ab){2,5}`, semver — untouched. False positives mean a server author must + * rewrite an avoidable nested quantifier; false negatives would let an attacker + * hang the host, so we err toward rejecting. + */ +export function looksCatastrophic(pattern: string): boolean { + let depth = 0; + // Per nesting level: does the group at this depth contain an unbounded + // quantifier somewhere inside it? + const groupHasUnbounded: boolean[] = []; + let i = 0; + while (i < pattern.length) { + const ch = pattern[i]!; + if (ch === "\\") { + // Escaped atom — skip the escape and the next char. + i += 2; + continue; + } + if (ch === "[") { + // Character class — skip to the matching ], honouring escapes. + i++; + while (i < pattern.length && pattern[i] !== "]") { + if (pattern[i] === "\\") i++; + i++; + } + i++; + continue; + } + if (ch === "(") { + depth++; + groupHasUnbounded[depth] = false; + i++; + continue; + } + if (ch === ")") { + const innerUnbounded = groupHasUnbounded[depth] === true; + const next = pattern[i + 1]; + let quantAmplifies = false; + if (next !== undefined && (next === "*" || next === "+")) { + quantAmplifies = true; // ) followed by * or + + } else if (next === "{") { + const close = pattern.indexOf("}", i + 1); + if (close > 0) { + const body = pattern.slice(i + 2, close); + if (/,\s*$/.test(body) || /,\s*\d{2,}\s*$/.test(body)) { + quantAmplifies = true; // {n,} open-ended or large upper bound + } else { + const m = /^(\d+)\s*(?:,\s*(\d+)\s*)?$/.exec(body); + if (m) { + const lo = Number(m[1]); + const hi = m[2] !== undefined ? Number(m[2]) : lo; + // {2}+ repetition of an unbounded-quantifier group → polynomial. + if (hi >= 2 || lo >= 2) quantAmplifies = true; + } + } + } + } + if (innerUnbounded && quantAmplifies) return true; + // An unbounded-quantified group bubbles its unboundedness to the parent. + if (next !== undefined && (next === "*" || next === "+") && depth - 1 > 0) { + groupHasUnbounded[depth - 1] = true; + } + if (depth > 0) depth--; + i++; + continue; + } + if (ch === "*" || ch === "+") { + if (depth > 0) groupHasUnbounded[depth] = true; + i++; + continue; + } + if (ch === "{") { + const close = pattern.indexOf("}", i); + if (close > 0) { + const body = pattern.slice(i + 1, close); + if (/,\s*$/.test(body) || /,\s*\d{2,}\s*$/.test(body)) { + if (depth > 0) groupHasUnbounded[depth] = true; + } + i = close + 1; + continue; + } + } + i++; + } + return false; +} + interface ForbiddenHit { label: string; index: number; @@ -298,3 +457,29 @@ export function attestSpawnStrict(signed: SignedManifest, request: SpawnRequest) ); } } + +/** + * Verify the manifest signature first, then attest the spawn. Fail-safe + * single call that closes the footgun where an *unverified* manifest is handed + * to `attestSpawn` — `attestSpawn` trusts that the caller already ran + * `verifyManifestStrict` at startup, but nothing structurally enforces it. + * + * Throws `AttestationError` with code `SIGNATURE_INVALID` if the signature + * does not verify, then the usual spawn-attestation codes if the request is + * not allowed. Re-verifying on every spawn costs one Ed25519 verify (~tens of + * microseconds); prefer this over bare `attestSpawnStrict` unless you have + * measured the verify out of a genuinely hot loop. + */ +export function attestSpawnVerified( + signed: SignedManifest, + request: SpawnRequest, + options: VerifyOptions = {}, +): void { + const result = verifyManifest(signed, options); + if (!result.valid) { + throw new AttestationError("SIGNATURE_INVALID", "Manifest verification failed before spawn attestation", { + errors: result.errors, + }); + } + attestSpawnStrict(signed, request); +} diff --git a/packages/lib/tests/fixtures/cve-2025-69256-payloads.json b/packages/lib/tests/fixtures/cve-2025-69256-payloads.json index 8d314f3..c6c1678 100644 --- a/packages/lib/tests/fixtures/cve-2025-69256-payloads.json +++ b/packages/lib/tests/fixtures/cve-2025-69256-payloads.json @@ -14,6 +14,9 @@ "redir>/dev/null", "redir { + const dangerous = [ + "(a+)+$", + "(a*)*", + "(.*)+", + "(.+)*", + "(a+)+", + "(\\d+)+$", + "([a-z]+)*$", + "(.*a){15}$", + "(x+x+)+y", + "((ab)*)*", + "(\\s+)*$", + "(a{1,})+", + ]; + const safe = [ + "^[a-z]+$", + "^\\d{1,10}$", + "^[a-zA-Z0-9_-]+$", + "abc", + "a*b*c*", + "^/safe/[a-z]+\\.txt$", + "(foo|bar)", + "[0-9]{4}-[0-9]{2}", + "\\w+@\\w+", + "^(https?)://", + "a+", + "^.{1,64}$", + "(abc)+", + "(a|b)+", + "(ab){2,5}", + "^v\\d+\\.\\d+\\.\\d+$", + "(cat|dog){1,3}", + ]; + + it.each(dangerous)("flags catastrophic pattern: %s", (p) => { + expect(looksCatastrophic(p)).toBe(true); + }); + + it.each(safe)("does NOT flag safe pattern (zero false positives): %s", (p) => { + expect(looksCatastrophic(p)).toBe(false); + }); + + it("the detector itself runs in linear time on a pathological pattern", () => { + // A long alternation of nested quantifiers — the detector must not itself + // backtrack. 2k chars should be sub-millisecond. + const pattern = "(a+)+".repeat(400); + const start = Date.now(); + expect(looksCatastrophic(pattern)).toBe(true); + expect(Date.now() - start).toBeLessThan(50); + }); +}); + +describe("A. ReDoS guard — evalArgRule fails closed on dangerous patterns", () => { + it("refuses to evaluate a catastrophic pattern (no hang) and returns a clear reason", () => { + const rule: ArgRule = { name: "x", kind: "regex", required: true, pattern: "(a+)+$" }; + const start = Date.now(); + // The classic exponential trigger input. Without the guard this hangs for + // tens of seconds; with it, it returns immediately. + const reasons = evalArgRule("a".repeat(40) + "!", rule); + expect(Date.now() - start).toBeLessThan(100); + expect(reasons.length).toBeGreaterThan(0); + expect(reasons.join(" ")).toMatch(/catastrophic backtracking/); + }); + + it("a safe regex still matches and rejects as before", () => { + const rule: ArgRule = { name: "x", kind: "regex", required: true, pattern: "^[a-z]+$" }; + expect(evalArgRule("hello", rule)).toEqual([]); + expect(evalArgRule("Hello", rule).length).toBeGreaterThan(0); + }); + + it("hard input-length cap (defense-in-depth) rejects over-long input before matching", () => { + const rule: ArgRule = { name: "x", kind: "regex", required: true, pattern: "^a+$", maxLength: 8 }; + expect(evalArgRule("aaaaaaaa", rule)).toEqual([]); // exactly 8 ok + const reasons = evalArgRule("aaaaaaaaa", rule); // 9 chars + expect(reasons.join(" ")).toMatch(/exceeds regex maxLength/); + }); + + it("attestSpawn does not freeze when a manifest ships a catastrophic regex", () => { + const { signed } = buildSigned([ + { + command: "/usr/bin/tool", + args: [{ name: "x", kind: "regex", required: true, pattern: "(a+)+$" }], + maxTotalArgLength: 4096, + }, + ]); + const start = Date.now(); + const r = attestSpawn(signed, { command: "/usr/bin/tool", args: ["a".repeat(45) + "!"] }); + expect(Date.now() - start).toBeLessThan(100); + expect(r.allowed).toBe(false); + expect(r.blockedReasons.join(" ")).toMatch(/catastrophic/); + }); +}); + +// --------------------------------------------------------------------------- +// B. Control-character gap (VT / FF / NEL) +// --------------------------------------------------------------------------- +describe("B. shellSafeString blocks VT / FF / NEL", () => { + const rule: SpawnRule = { + command: "/x", + args: [{ name: "a", kind: "shellSafeString", required: true, maxLength: 256 }], + maxTotalArgLength: 1024, + }; + + it.each([ + ["VT U+000B", "a b"], + ["FF U+000C", "a b"], + ["NEL U+0085", "a…b"], + ])("blocks %s", (_label, payload) => { + const r = sanitizeArgs([payload], rule); + expect(r.allowed).toBe(false); + expect(r.blockedReasons.join(" ")).toMatch(/forbidden character/); + }); + + it("still allows a plain space-separated value (no over-reach)", () => { + expect(sanitizeArgs(["hello world"], rule).allowed).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// C. Prefix path-traversal +// --------------------------------------------------------------------------- +describe("C. containsTraversal helper", () => { + it.each([ + "/safe/../etc/passwd", + "/safe/../../etc/passwd", + "..", + "../x", + "x/..", + "a/../b", + "a\\..\\b", + "/safe/%2e%2e/etc/passwd", + "/safe/%2E%2E/etc/passwd", + ])("detects traversal in: %s", (v) => { + expect(containsTraversal(v)).toBe(true); + }); + + it.each([ + "/safe/file.txt", + "/safe/sub/dir/file", + "file..name", // `..` embedded in a token, not a path component + "version.2.0", + "a..b", // not bounded by separators + "/safe/...hidden", // three dots is not a `..` component + ])("does NOT flag legitimate value: %s", (v) => { + expect(containsTraversal(v)).toBe(false); + }); +}); + +describe("C. prefix rule rejects traversal by default, allows clean paths", () => { + it("ATTACK: /safe/../../etc/passwd is blocked even though it satisfies the prefix", () => { + const { signed } = buildSigned([ + { + command: "/usr/bin/cat", + args: [{ name: "file", kind: "prefix", required: true, prefix: "/safe/", maxSuffixLength: 256 }], + maxTotalArgLength: 1024, + }, + ]); + const r = attestSpawn(signed, { command: "/usr/bin/cat", args: ["/safe/../../etc/passwd"] }); + expect(r.allowed).toBe(false); + expect(r.blockedReasons.join(" ")).toMatch(/path-traversal/); + }); + + it("BENIGN: a clean path under the prefix is still allowed", () => { + const { signed } = buildSigned([ + { + command: "/usr/bin/cat", + args: [{ name: "file", kind: "prefix", required: true, prefix: "/safe/", maxSuffixLength: 256 }], + maxTotalArgLength: 1024, + }, + ]); + const r = attestSpawn(signed, { command: "/usr/bin/cat", args: ["/safe/reports/q1.txt"] }); + expect(r.allowed).toBe(true); + }); + + it("OPT-OUT: denyTraversal:false restores legacy startsWith-only behaviour", () => { + const reasons = evalArgRule("/safe/../x", { + name: "file", + kind: "prefix", + required: true, + prefix: "/safe/", + maxSuffixLength: 256, + denyTraversal: false, + }); + expect(reasons).toEqual([]); + }); + + it("missing-prefix is still reported (no behaviour drift)", () => { + const reasons = evalArgRule("/etc/passwd", { + name: "file", + kind: "prefix", + required: true, + prefix: "/safe/", + maxSuffixLength: 256, + }); + expect(reasons.join(" ")).toMatch(/missing required prefix/); + }); +}); + +// --------------------------------------------------------------------------- +// D. attestSpawnVerified +// --------------------------------------------------------------------------- +describe("D. attestSpawnVerified — verify-then-attest fail-safe", () => { + function fixture() { + return buildSigned([ + { + command: "/usr/bin/echo", + args: [{ name: "m", kind: "shellSafeString", required: true, maxLength: 256 }], + maxTotalArgLength: 1024, + }, + ]); + } + + it("passes for a valid manifest + allowed request", () => { + const { signed } = fixture(); + expect(() => attestSpawnVerified(signed, { command: "/usr/bin/echo", args: ["hello"] })).not.toThrow(); + }); + + it("ATTACK: a tampered manifest (extra spawn rule, not re-signed) is rejected at the signature step", () => { + const { signed } = fixture(); + const tampered = { + ...signed, + manifest: { + ...signed.manifest, + spawnRules: [ + ...signed.manifest.spawnRules, + { command: "/bin/bash", args: [], maxTotalArgLength: 1024 }, + ], + }, + } as typeof signed; + + // Bare attestSpawn would wrongly allow it (documented footgun) ... + expect(attestSpawn(tampered, { command: "/bin/bash", args: [] }).allowed).toBe(true); + // ... but the verified variant fails closed on the broken signature. + let thrown: unknown; + try { + attestSpawnVerified(tampered, { command: "/bin/bash", args: [] }); + } catch (e) { + thrown = e; + } + expect(thrown).toBeInstanceOf(AttestationError); + expect((thrown as AttestationError).code).toBe("SIGNATURE_INVALID"); + }); + + it("throws the usual spawn codes for a valid manifest but disallowed command", () => { + const { signed } = fixture(); + let thrown: unknown; + try { + attestSpawnVerified(signed, { command: "/bin/bash", args: ["-c", "ls"] }); + } catch (e) { + thrown = e; + } + expect(thrown).toBeInstanceOf(AttestationError); + expect((thrown as AttestationError).code).toBe("SPAWN_COMMAND_NOT_ALLOWED"); + }); + + it("honours the verify `now` option (expired manifest rejected before attest)", () => { + const kp = generateKeyPair(); + const tpl = generateTemplate({ serverName: "harden-test", toolNames: ["t"] }); + const m: Manifest = { + ...tpl, + publicKeyFingerprint: kp.fingerprint, + signer: "harden-tester", + signedAt: "2026-04-27T00:00:00.000Z", + expiresAt: "2026-04-28T00:00:00.000Z", + spawnRules: [ + { command: "/usr/bin/echo", args: [{ name: "m", kind: "shellSafeString", required: true, maxLength: 256 }], maxTotalArgLength: 1024 }, + ], + }; + const signed = signManifest(m, kp.privateKeyHex); + expect(() => + attestSpawnVerified(signed, { command: "/usr/bin/echo", args: ["ok"] }, { now: new Date("2026-04-29T00:00:00Z") }), + ).toThrow(AttestationError); + }); +});