Skip to content

[CLAUDE ROUTINE]: Security enhancement — bound the quantifiers in LOCAL_IMPORT_RE / LOCAL_REQUIRE_RE so a hand-crafted policy file can't ReDoS the custom-hook loader #249

Description

@NiveditJain

Summary

The custom-hook loader scans every reachable policy file with two regexes that contain unbounded [\s\S]*? segments. They work great on real-world import / require lines, but on pathological input (a single 100 KB line with many quote characters) the engine can backtrack catastrophically. This is the classic ReDoS shape, and since we run this regex over user-authored .js files (which we already invite users to write), bounding the quantifier removes a sharp edge before it cuts anyone.

Note: this is distinct from #137, which targets user-supplied patterns inside sanitize-api-keys. The issue here is in our own code that parses user policy files.

Where

src/hooks/loader-utils.ts:21,24

/** Regex to find local relative import specifiers (ESM). */
const LOCAL_IMPORT_RE = /(?:import\s+(?:[\s\S]*?\s+from\s+)?|export\s+(?:[\s\S]*?\s+from\s+))(['"])(\.\.?\/[^'"]+)\1/g;

/** Regex to find local relative require specifiers (CJS). */
const LOCAL_REQUIRE_RE = /require\s*\(\s*(['"])(\.\.?\/[^'"]+)\1\s*\)/g;

LOCAL_IMPORT_RE is the riskier one — [\s\S]*?\s+from\s+ is lazy but the engine still has to try every backtrack point if from never appears on a long line. LOCAL_REQUIRE_RE is shape-bounded but worth tightening for symmetry.

Used by rewriteFileTree() (src/hooks/custom-hooks-loader.ts) — invoked on every hook event when custom policies are configured.

Why this helps users

sequenceDiagram
    participant CC as Claude Code
    participant H as Hook handler
    participant L as Custom hook loader
    participant FS as Disk
    CC->>H: PreToolUse event
    H->>L: rewriteFileTree(entry)
    L->>FS: read user policy.js
    FS-->>L: 80 KB single-line file
    L->>L: LOCAL_IMPORT_RE.exec(code)
    Note over L: Backtracking explosion\nhandler exceeds 10s timeout\n→ falls back to "allow"
    L-->>H: timeout
    H-->>CC: allow (silently degraded)
Loading
  • Predictable hook latency[\s\S]{0,N}? keeps worst-case parse linear in line length.
  • Loader stays correct on minified files — ironically, a minified bundled policy can already trip this if a user copy-pastes one.
  • No regressionimport ... from "..." statements are short by design; a small ceiling (e.g. 2 KB) is well above realistic specifiers.

Proposed enhancement

  1. Replace the inner unbounded quantifier with a bounded one that comfortably covers real imports:
    const MAX_IMPORT_HEAD = 2048;
    const LOCAL_IMPORT_RE = new RegExp(
      `(?:import\\s+(?:[\\s\\S]{0,${MAX_IMPORT_HEAD}}?\\s+from\\s+)?|export\\s+(?:[\\s\\S]{0,${MAX_IMPORT_HEAD}}?\\s+from\\s+))(['"])(\\.\\.?\\/[^'"]+)\\1`,
      "g",
    );
  2. Add a one-liner length guard that skips the regex pass entirely if a file is suspiciously large:
    if (code.length > MAX_POLICY_FILE_BYTES) { /* warn + skip rewrite, fail-open */ }
  3. Add a focused unit test under __tests__/hooks/loader-utils.test.ts:
    • Normal ESM/CJS imports still match (regression).
    • 64 KB single line of ' and " characters returns within 50 ms.

Acceptance criteria

  • Bounded quantifier in both LOCAL_IMPORT_RE and LOCAL_REQUIRE_RE.
  • Optional length guard with hookLogWarn so power users see why their giant generated file was skipped.
  • Unit test with timing assertion (expect(duration).toBeLessThan(50)).
  • CHANGELOG.md entry under Unreleased → Fixes.

Severity

Low — exploitable only by self-authored policy files (which already run arbitrary code), but cheap to fix and removes a foot-gun on copy-pasted/minified files.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions