Skip to content

fix: security audit — config injection, ReDoS, confidence consistency, bounds#4

Merged
ether-btc merged 1 commit intomasterfrom
fix/security-audit-2026-04-27
May 4, 2026
Merged

fix: security audit — config injection, ReDoS, confidence consistency, bounds#4
ether-btc merged 1 commit intomasterfrom
fix/security-audit-2026-04-27

Conversation

@ether-btc
Copy link
Copy Markdown
Owner

Summary

Security and correctness audit of scripts/correlation-memory.ts via MiniMax-M2.7. 5 issues fixed.


CRITICAL — Config-driven path injection (RCE-adjacent)

File: scripts/correlation-memory.ts

Vulnerability: resolveWorkspacePath() fell back to api.config?.agents?.defaults?.workspace. If an attacker could influence the OpenClaw API config, they could redirect correlation-rules.json loading to an attacker-controlled file — enabling rule injection, ReDoS, or decision manipulation.

Fix: Removed the config fallback entirely. Now uses only ctx.workspaceDir (SDK-provided, trusted) and a hardcoded safe default.


HIGH — ReDoS via pathological keywords

File: scripts/correlation-memory.ts (lines 130-155)

Vulnerability: Keywords from correlation-rules.json were compiled directly to RegExp without complexity limits. Even with character escaping, patterns like a{100} can cause exponential backtracking on long input text.

Fix:

  • Simple alphanumeric keywords now use String.includes() — O(n*m), no backtracking
  • Keywords >100 chars rejected before escaping
  • Regex still used for keywords with special characters

HIGH — Confidence validation inconsistency

File: scripts/correlation-memory.ts (lines 73-77 vs 200-204)

Bug: loadCorrelationRules kept rules with confidence: undefined but matchRules filtered them out. Result: undefined-confidence rules silently passed load but were dropped at match time.

Fix: Explicit undefined check in load filter makes behavior consistent.


MEDIUM — Missing Unicode flag

File: scripts/correlation-memory.ts (line 141)

Bug: Word-boundary regex has undefined behavior for non-ASCII characters without the u flag.

Fix: Changed "i" to "iu" (case-insensitive + Unicode).


MEDIUM — No upper bound on max_results

File: scripts/correlation-memory.ts (lines 369-371, 452-454)

Bug: No ceiling on max_results — a caller could pass Number.MAX_SAFE_INTEGER, causing unbounded array allocation.

Fix: Added Math.min(1000, Math.max(1, floor(...))) ceiling.


Build fix

  • package.json build script pointed to deleted index.ts — updated to scripts/correlation-memory.ts

Test results

✓ tests/correlation.test.ts (26 tests) 49ms
Test Files  1 passed (1)
     Tests  26 passed (26)

Auditor: MiniMax-M2.7 (gateway)
Model used for delegation: MiniMax-M2.7

…stency, bounds

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).
@ether-btc ether-btc merged commit 2e2b28f into master May 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant