← Back to PRs

#23670: fix(security): add ReDoS protection for user-controlled regex patterns

by kevinWangSheng open 2026-02-22 15:26 View on GitHub →
channel: discord size: S
## Summary - Problem: User-controlled regex patterns in `sessionFilter` config are compiled via `new RegExp()` without complexity validation, allowing catastrophic backtracking (ReDoS) - Why it matters: A misconfigured or malicious sessionFilter pattern could hang the gateway event loop, causing denial of service - What changed: Added `isSafeRegexPattern()` guard that rejects overly long patterns and patterns with nested quantifiers before `new RegExp()` construction - What did NOT change: Existing valid regex patterns continue to work; substring matching fallback is unchanged ## Change Type - [x] Security hardening ## Scope - [x] Gateway / orchestration ## Security Impact - New permissions/capabilities? No - Secrets/tokens handling changed? No - New/changed network calls? No - Command/tool execution surface changed? No - Data access scope changed? No ## Compatibility / Migration - Backward compatible? Yes - Config/env changes? No - Migration needed? No ## Risks and Mitigations - Risk: Some valid but complex regex patterns in sessionFilter may be rejected - Mitigation: 200-char limit is generous; nested quantifier check targets the most common ReDoS pattern ## Test plan - [x] Added unit tests for `isSafeRegexPattern()` covering safe patterns, long patterns, and various nested quantifier forms - [x] Verified existing exec approval forwarder tests still pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> Added ReDoS protection for user-controlled regex patterns in `sessionFilter` configurations by implementing `isSafeRegexPattern()` guard function. The guard rejects patterns longer than 200 characters and patterns with nested quantifiers before `new RegExp()` construction. **Key changes:** - Created `isSafeRegexPattern()` function that validates regex patterns for ReDoS risk - Applied protection in both `matchSessionFilter()` (infra) and Discord `shouldHandle()` method - Falls back to substring matching for unsafe patterns instead of rejecting them - Added comprehensive unit tests covering length limits and various nested quantifier forms **Issues found:** - Non-greedy nested quantifiers like `(a+?)+` may bypass detection (see inline comment) **Note:** Despite the edge case, this is a solid security improvement that protects against the most common ReDoS patterns. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with one minor edge case noted - Strong security fix with comprehensive tests. The nested quantifier detection has one edge case (non-greedy quantifiers), but this is unlikely to be exploited in practice since sessionFilter patterns rarely use non-greedy quantifiers. The fix properly handles both usage locations, maintains backward compatibility with fallback to substring matching, and significantly reduces ReDoS risk. - Pay attention to src/infra/exec-approval-forwarder.ts:62 where the nested quantifier detection could be strengthened <sub>Last reviewed commit: 684382e</sub> <!-- greptile_other_comments_section --> <sub>(5/5) You can turn off certain types of comments like style [here](https://app.greptile.com/review/github)!</sub> <!-- /greptile_comment -->

Most Similar PRs