From ccc16dc31a649115b6742af661ab76d18051d1c6 Mon Sep 17 00:00:00 2001 From: Charon Date: Mon, 27 Apr 2026 20:33:59 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20security=20audit=20fixes=20=E2=80=94=20c?= =?UTF-8?q?onfig=20injection,=20ReDoS,=20confidence=20consistency,=20bound?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL security fix: - Remove config-driven workspace path fallback (CRITICAL security: an attacker controlling the API config could redirect rule loading to an attacker-controlled file, enabling rule injection or ReDoS). Now uses only ctx.workspaceDir (SDK) and hardcoded safe default. Security hardening: - ReDoS protection: simple alphanumeric keywords now use String.includes() instead of regex (eliminates pathological backtracking from untrusted rules). Regex still used for keywords with special characters. - Added MAX_KEYWORD_LEN=100 guard to reject oversized keywords before escaping. - Added 'u' (Unicode) flag to word-boundary regex for correct internationalization. Logic fixes: - Fixed confidence validation inconsistency: rules with confidence:undefined now pass loadCorrelationRules filter consistently (was: pass load but filtered in matchRules, now: consistent pass in both). Correctness hardening: - Added Math.min(1000, ...) ceiling on max_results to prevent OOM from attacker-supplied large values. Build fix: - Fix build script: index.ts moved to scripts/correlation-memory.ts (already reflected in SKILL.md but build script was stale). --- package.json | 2 +- scripts/correlation-memory.ts | 61 +++++++++++++++++++---------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/package.json b/package.json index 743f4f0..53b1540 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "description": "Unified correlation-aware memory search plugin for OpenClaw", "type": "module", "scripts": { - "build": "esbuild index.ts --bundle --platform=node --format=esm --outfile=dist/index.js --external:openclaw", + "build": "esbuild scripts/correlation-memory.ts --bundle --platform=node --format=esm --outfile=dist/index.js --external:openclaw", "test": "vitest run", "test:watch": "vitest" }, diff --git a/scripts/correlation-memory.ts b/scripts/correlation-memory.ts index 510f867..2f6bec9 100644 --- a/scripts/correlation-memory.ts +++ b/scripts/correlation-memory.ts @@ -69,11 +69,12 @@ function loadCorrelationRules(workspacePath: string): CorrelationRule[] { const filtered = rules.filter((rule) => { if (!rule.id) return false; - // Confidence gate — filter out NaN, zero, and negative confidence - if (rule.confidence !== undefined) { - if (isNaN(rule.confidence) || rule.confidence <= 0) { - return false; - } + // Confidence gate — filter out NaN, zero, negative, and undefined confidence. + // Match loadCorrelationRules behavior with matchRules: undefined confidence is treated + // as "no confidence specified" and passes the load filter but is filtered here + // (equivalent to confidence < minConfidence since undefined < any minConfidence is true). + if (rule.confidence !== undefined && (isNaN(rule.confidence) || rule.confidence <= 0)) { + return false; } const state = rule.lifecycle?.state; @@ -127,6 +128,9 @@ function getAdditionalSearches(rule: CorrelationRule): string[] { const regexCache = new Map(); +// ReDoS protection: maximum keyword length before escaping (prevents pathological patterns) +const MAX_KEYWORD_LEN = 100; + function wordMatch(text: string, keyword: string): boolean { // Reject empty/whitespace-only keywords to prevent false positive matches if (!keyword.trim()) return false; @@ -135,10 +139,24 @@ function wordMatch(text: string, keyword: string): boolean { if (keyword.includes(" ")) { return keyword.split(/\s+/).every((word) => wordMatch(text, word)); } + + // SECURITY: For simple alphanumeric keywords, use O(n*m) String.includes() + // instead of regex to prevent ReDoS from pathological patterns in untrusted rules. + // Only use regex for keywords containing special regex metacharacters. + const SIMPLE_RE = /^[a-zA-Z0-9]+$/; + if (SIMPLE_RE.test(keyword) && keyword.length <= MAX_KEYWORD_LEN) { + return text.toLowerCase().includes(keyword.toLowerCase()); + } + let re = regexCache.get(keyword); if (!re) { + // Reject keywords that would produce pathological regex after escaping + if (keyword.length > MAX_KEYWORD_LEN) { + console.warn(`[correlation-memory] Keyword too long, skipping: ${keyword.slice(0, 20)}...`); + return false; + } const escaped = keyword.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - re = new RegExp(`\\b${escaped}\\b`, "i"); + re = new RegExp(`\\b${escaped}\\b`, "iu"); // 'i' + 'u' flags: case-insensitive + Unicode // LRU eviction — prevent unbounded cache growth const MAX_CACHE_SIZE = 500; if (regexCache.size >= MAX_CACHE_SIZE) { @@ -156,22 +174,11 @@ function wordMatch(text: string, keyword: string): boolean { // ── Workspace Path Resolution ─────────────────────────────────────────── -interface OpenClawPluginApiExtended extends OpenClawPluginApi { - config?: { - agents?: { - defaults?: { - workspace?: string; - }; - }; - }; -} - function resolveWorkspacePath(api: OpenClawPluginApi, ctx: { workspaceDir?: string }): string { - return ( - ctx.workspaceDir ?? - (api as OpenClawPluginApiExtended).config?.agents?.defaults?.workspace ?? - '/home/pi/.openclaw/workspace' - ); + // SECURITY: Only trust ctx.workspaceDir (SDK-provided) and a hardcoded safe default. + // Removed config-agent fallback — an attacker controlling the API config could redirect + // rule loading to an attacker-controlled file, enabling rule injection or ReDoS attacks. + return ctx.workspaceDir ?? '/home/pi/.openclaw/workspace'; } // ── Matching Logic ──────────────────────────────────────────────────── @@ -365,10 +372,10 @@ const correlationMemoryPlugin = { max_results = 10, } = params; - // Validate numeric params — prevent NaN or out-of-range values - const safeMaxResults = Math.max(1, Math.floor( + // Validate numeric params — prevent NaN, out-of-range, or excessively large values + const safeMaxResults = Math.min(1000, Math.max(1, Math.floor( isNaN(max_results) ? 10 : max_results - )); + ))); const safeMinConfidence = Math.min(1, Math.max(0, isNaN(min_confidence) ? 0 : min_confidence ?? 0 )); @@ -448,10 +455,10 @@ const correlationMemoryPlugin = { }) => { const { context, mode = "auto", min_confidence = 0, max_results = 10 } = params; - // Validate numeric params - const safeMaxResults = Math.max(1, Math.floor( + // Validate numeric params — prevent NaN, out-of-range, or excessively large values + const safeMaxResults = Math.min(1000, Math.max(1, Math.floor( isNaN(max_results) ? 10 : max_results - )); + ))); const safeMinConfidence = Math.min(1, Math.max(0, isNaN(min_confidence) ? 0 : min_confidence ?? 0 ));