Skip to content

fix(entity): defuse entity-candidate ReDoS on long ASCII runs (#2063) - #2065

Closed
mvalentsev wants to merge 1 commit into
MemPalace:developfrom
mvalentsev:fix/2063-entity-redos
Closed

fix(entity): defuse entity-candidate ReDoS on long ASCII runs (#2063)#2065
mvalentsev wants to merge 1 commit into
MemPalace:developfrom
mvalentsev:fix/2063-entity-redos

Conversation

@mvalentsev

Copy link
Copy Markdown
Contributor

Fixes #2063.

What does this PR do?

mempalace mine can pin a CPU core for hours (39h reported) on a single entity-extraction window. The English entity-candidate pattern in i18n/en.json ([A-Z][a-z]+(?:[A-Z][a-z]+|[A-Z]{2,})+|[A-Z][a-z]{1,19}) backtracks catastrophically on a long unbroken run of printable ASCII — base64, minified JS, hashes, data URIs — which turn up routinely in transcripts and code files. On develop, "Aa" + "B"*36 already takes ~3s inside findall, and a real ~5000-char base64 window never returns. The pattern is compiled and run in two places — palace._candidate_entity_words (the traceback in the issue) and entity_detector.extract_candidates — and both hang.

Possessive quantifiers would fix the regex directly, but they're 3.11+ and the project supports 3.9 (pattern compilation is wrapped in try/except re.error: continue, so a 3.11-only construct would silently disable English extraction on older Pythons). Instead this collapses long unbroken ASCII runs to a space before single-word candidate matching, in both places. Such a run is never a name, so real entity output is unchanged.

The collapse is restricted to ASCII ([!-~]) on purpose: base64/hashes are always ASCII, so it still catches every reported case, but it leaves CJK, Cyrillic, Devanagari and accented-Latin text alone. A blanket \S version would have wiped out Chinese detection — Chinese has no ASCII whitespace, so a whole paragraph is one unbroken run, and zh-CN/zh-TW have no multi-word fallback. The threshold (24) sits just above the 20-char cap of the simple-name pattern ([A-Z][a-z]{1,19}), so no ordinary name is affected. Only en.json's candidate pattern has the ambiguous nested-repetition shape, so other locales can't regress either.

How to test

uv run pytest tests/test_entity_detector.py tests/test_palace.py -v

Before this PR the following never returns; after, it's instant:

from mempalace.entity_detector import extract_candidates
extract_candidates("Aa" + "B" * 400 + "0")

New tests cover the collapse and its 23/24 boundary, that non-ASCII runs are never collapsed, that a Chinese paragraph still yields entities (zh regression guard), a hypothesis property that no input can leave a collapsible run, and a subprocess/timeout test that runs the real payload through both consumers so a regression fails fast instead of hanging (there's no pytest-timeout here).

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

Based on @RyanWei's proposed fix in #2063, extended to both call sites, scoped to ASCII so CJK detection keeps working, with the threshold lowered to 24.

…ace#2063)

The English entity-candidate pattern in i18n/en.json backtracks
catastrophically on a long unbroken run of printable ASCII (base64,
minified JS, hashes, data URIs), pinning `mempalace mine` on a single
~5000-char window for hours. Collapse such runs to a space before
single-word candidate matching, in both consumers that apply the pattern
(palace._candidate_entity_words and entity_detector.extract_candidates).

Scoped to ASCII ([!-~]) so non-ASCII scripts stay untouched — a CJK
paragraph is one unbroken run with no ASCII whitespace, and zh-CN/zh-TW
have no multi-word fallback, so a blanket collapse would erase their
detection. Threshold 24 sits above the 20-char cap of the simple-name
pattern, so no real name is dropped.

Fixes MemPalace#2063.

Co-authored-by: Ryan Wei <9876551+RyanWei@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Entity extraction regex backtracks catastrophically on base64/minified content — mines pinned for days

1 participant