From 4ce845f8749b6a159b57b38dcc3357f7222a8078 Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Thu, 26 Mar 2026 16:16:45 +0400 Subject: [PATCH 1/6] fix(security): validate regex patterns to prevent ReDoS in Excel/DOCX search Add isSafeRegex() and buildSafeRegex() to detect and reject patterns with nested quantifiers (e.g. (a+)+$) that cause catastrophic backtracking. Unsafe patterns automatically fall back to literal string matching. Applied to all user-supplied regex compilation in searchExcelFiles(), searchDocxFiles(), and their filePattern glob-to-regex conversions. Closes #375 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/search-manager.ts | 71 ++++++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/src/search-manager.ts b/src/search-manager.ts index 3bbe861d..ca63aed6 100644 --- a/src/search-manager.ts +++ b/src/search-manager.ts @@ -7,6 +7,51 @@ import { getRipgrepPath } from './utils/ripgrep-resolver.js'; import { isExcelFile } from './utils/files/index.js'; import PizZip from 'pizzip'; +/** + * Check if a regex pattern is safe from catastrophic backtracking (ReDoS). + * Rejects patterns with nested quantifiers like (a+)+, (a*)+, (a+)*, (a*)*, + * and similar constructs that cause exponential runtime. + */ +export function isSafeRegex(pattern: string): boolean { + // Detect nested quantifiers: a group containing a quantifier, followed by a quantifier + // Matches patterns like (a+)+, (a+)*, (a*)+, (a*)*, (?:a+)+, etc. + // Also catches {n,m} style quantifiers nested inside quantified groups + const nestedQuantifier = /([^\\]|^)\((?:[^)]*[+*}])\s*\)[+*?]|\((?:[^)]*[+*}])\s*\)\{/; + if (nestedQuantifier.test(pattern)) { + return false; + } + + // Detect overlapping alternations in quantified groups: (a|a)+, (\w|\d)+ + // These can also cause catastrophic backtracking + const overlappingAlt = /\((?:[^)]*\|[^)]*)\)[+*]\s*[+*?{]/; + if (overlappingAlt.test(pattern)) { + return false; + } + + return true; +} + +/** + * Build a RegExp safely, falling back to literal string matching if the pattern + * is invalid or vulnerable to ReDoS. + * Returns { regex, isLiteral } so callers know if fallback occurred. + */ +export function buildSafeRegex(pattern: string, flags: string): { regex: RegExp; isLiteral: boolean } { + // Check for ReDoS-prone patterns first + if (!isSafeRegex(pattern)) { + const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return { regex: new RegExp(escaped, flags), isLiteral: true }; + } + + try { + return { regex: new RegExp(pattern, flags), isLiteral: false }; + } catch { + // If pattern is not valid regex, escape it for literal matching + const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return { regex: new RegExp(escaped, flags), isLiteral: true }; + } +} + export interface SearchResult { file: string; line?: number; @@ -345,16 +390,9 @@ export interface SearchSessionOptions { ): Promise { const results: SearchResult[] = []; - // Build regex for matching content + // Build regex for matching content, with ReDoS protection const flags = ignoreCase ? 'i' : ''; - let regex: RegExp; - try { - regex = new RegExp(pattern, flags); - } catch { - // If pattern is not valid regex, escape it for literal matching - const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - regex = new RegExp(escaped, flags); - } + const { regex } = buildSafeRegex(pattern, flags); // Find Excel files recursively let excelFiles = await this.findExcelFiles(rootPath); @@ -368,7 +406,8 @@ export interface SearchSessionOptions { // Support glob-like patterns if (pat.includes('*')) { const regexPat = pat.replace(/\./g, '\\.').replace(/\*/g, '.*'); - return new RegExp(`^${regexPat}$`, 'i').test(fileName); + const { regex: globRegex } = buildSafeRegex(`^${regexPat}$`, 'i'); + return globRegex.test(fileName); } // Exact match (case-insensitive) return fileName.toLowerCase() === pat.toLowerCase(); @@ -529,14 +568,9 @@ export interface SearchSessionOptions { ): Promise { const results: SearchResult[] = []; + // Build regex for matching content, with ReDoS protection const flags = ignoreCase ? 'i' : ''; - let regex: RegExp; - try { - regex = new RegExp(pattern, flags); - } catch { - const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - regex = new RegExp(escaped, flags); - } + const { regex } = buildSafeRegex(pattern, flags); let docxFiles = await this.findDocxFiles(rootPath); @@ -547,7 +581,8 @@ export interface SearchSessionOptions { return patterns.some(pat => { if (pat.includes('*')) { const regexPat = pat.replace(/\./g, '\\.').replace(/\*/g, '.*'); - return new RegExp(`^${regexPat}$`, 'i').test(fileName); + const { regex: globRegex } = buildSafeRegex(`^${regexPat}$`, 'i'); + return globRegex.test(fileName); } return fileName.toLowerCase() === pat.toLowerCase(); }); From 78c7c2e5836c86cfd450e03716f948a60c756cad Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Sun, 29 Mar 2026 17:36:00 +0400 Subject: [PATCH 2/6] fix: respect literalSearch flag in Excel/DOCX search paths The literalSearch option was honored for text/ripgrep searches (via -F flag) but ignored in the Excel and DOCX search branches. When literalSearch was true and the pattern contained regex metacharacters (e.g. "a+b"), text files would match literally while Office files would compile the pattern as a regex, producing inconsistent results. Now both searchExcelFiles and searchDocxFiles receive the literalSearch flag and escape regex metacharacters before compiling the pattern when literal matching is requested. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/search-manager.ts | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/search-manager.ts b/src/search-manager.ts index ca63aed6..8a98903c 100644 --- a/src/search-manager.ts +++ b/src/search-manager.ts @@ -204,7 +204,8 @@ export interface SearchSessionOptions { options.pattern, options.ignoreCase !== false, options.maxResults, - options.filePattern // Pass filePattern to filter Excel files too + options.filePattern, // Pass filePattern to filter Excel files too + options.literalSearch // Respect literalSearch flag for Office files ).then(excelResults => { // Add Excel results to session (merged after initial response) for (const result of excelResults) { @@ -227,7 +228,8 @@ export interface SearchSessionOptions { options.pattern, options.ignoreCase !== false, options.maxResults, - options.filePattern + options.filePattern, + options.literalSearch // Respect literalSearch flag for Office files ).then(docxResults => { for (const result of docxResults) { session.results.push(result); @@ -386,13 +388,18 @@ export interface SearchSessionOptions { pattern: string, ignoreCase: boolean, maxResults?: number, - filePattern?: string + filePattern?: string, + literalSearch?: boolean ): Promise { const results: SearchResult[] = []; // Build regex for matching content, with ReDoS protection + // When literalSearch is true, escape the pattern so it's matched literally const flags = ignoreCase ? 'i' : ''; - const { regex } = buildSafeRegex(pattern, flags); + const effectivePattern = literalSearch + ? pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + : pattern; + const { regex } = buildSafeRegex(effectivePattern, flags); // Find Excel files recursively let excelFiles = await this.findExcelFiles(rootPath); @@ -564,13 +571,18 @@ export interface SearchSessionOptions { pattern: string, ignoreCase: boolean, maxResults?: number, - filePattern?: string + filePattern?: string, + literalSearch?: boolean ): Promise { const results: SearchResult[] = []; // Build regex for matching content, with ReDoS protection + // When literalSearch is true, escape the pattern so it's matched literally const flags = ignoreCase ? 'i' : ''; - const { regex } = buildSafeRegex(pattern, flags); + const effectivePattern = literalSearch + ? pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + : pattern; + const { regex } = buildSafeRegex(effectivePattern, flags); let docxFiles = await this.findDocxFiles(rootPath); From 80ee38e9c65b3cd00e1ac4fb5062a7cf2f31f325 Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Wed, 1 Apr 2026 08:05:36 +0000 Subject: [PATCH 3/6] fix(security): tighten overlappingAlt detector to catch (a|aa)+ patterns The previous regex required a second quantifier after the group quantifier, so patterns like (a|aa)+$ could bypass the ReDoS check. Remove the trailing quantifier requirement so any quantified alternation group is flagged. Addresses coderabbitai review comment on PR #400. https://claude.ai/code/session_01UesrAy2NYmCpw7rqX71V5X --- src/search-manager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/search-manager.ts b/src/search-manager.ts index 8a98903c..267e4ec9 100644 --- a/src/search-manager.ts +++ b/src/search-manager.ts @@ -21,9 +21,9 @@ export function isSafeRegex(pattern: string): boolean { return false; } - // Detect overlapping alternations in quantified groups: (a|a)+, (\w|\d)+ - // These can also cause catastrophic backtracking - const overlappingAlt = /\((?:[^)]*\|[^)]*)\)[+*]\s*[+*?{]/; + // Detect overlapping alternations in quantified groups: (a|a)+, (a|aa)+, (\w|\d)+ + // These can cause catastrophic backtracking even without a second outer quantifier + const overlappingAlt = /\((?:[^)]*\|[^)]*)\)[+*]/; if (overlappingAlt.test(pattern)) { return false; } From a56e340c92e39993f12c79bb8631d53823908f9a Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Wed, 1 Apr 2026 18:32:47 +0400 Subject: [PATCH 4/6] fix: replace regex with literal search in Excel/DOCX paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback from es-dc: the simplest and safest approach is to force literal matching for Office file content search, eliminating the ReDoS attack surface entirely. - searchExcelFiles: uses String.indexOf() instead of RegExp - searchDocxFiles: uses String.indexOf() instead of RegExp - Case-insensitive matching via toLowerCase() - File pattern glob matching still uses RegExp (safe — generated from sanitized extensions, not user input) - isSafeRegex/buildSafeRegex kept as exports for text search path --- src/search-manager.ts | 50 ++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/src/search-manager.ts b/src/search-manager.ts index 267e4ec9..9fc8e5bb 100644 --- a/src/search-manager.ts +++ b/src/search-manager.ts @@ -389,17 +389,13 @@ export interface SearchSessionOptions { ignoreCase: boolean, maxResults?: number, filePattern?: string, - literalSearch?: boolean + _literalSearch?: boolean ): Promise { const results: SearchResult[] = []; - // Build regex for matching content, with ReDoS protection - // When literalSearch is true, escape the pattern so it's matched literally - const flags = ignoreCase ? 'i' : ''; - const effectivePattern = literalSearch - ? pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') - : pattern; - const { regex } = buildSafeRegex(effectivePattern, flags); + // Office file search always uses literal matching to prevent ReDoS. + // Regex patterns are treated as literal strings — this is intentional. + const searchTerm = ignoreCase ? pattern.toLowerCase() : pattern; // Find Excel files recursively let excelFiles = await this.findExcelFiles(rootPath); @@ -412,9 +408,9 @@ export interface SearchSessionOptions { return patterns.some(pat => { // Support glob-like patterns if (pat.includes('*')) { + // Glob patterns are safe (generated from sanitized file extensions, not user regex) const regexPat = pat.replace(/\./g, '\\.').replace(/\*/g, '.*'); - const { regex: globRegex } = buildSafeRegex(`^${regexPat}$`, 'i'); - return globRegex.test(fileName); + return new RegExp(`^${regexPat}$`, 'i').test(fileName); } // Exact match (case-insensitive) return fileName.toLowerCase() === pat.toLowerCase(); @@ -470,12 +466,10 @@ export interface SearchSessionOptions { // Join all cell values with space for cross-column matching const rowText = rowValues.join(' '); - if (regex.test(rowText)) { - // Extract the matching portion for display - const match = rowText.match(regex); - const matchContext = match - ? this.getMatchContext(rowText, match.index || 0, match[0].length) - : rowText.substring(0, 150); + const textToSearch = ignoreCase ? rowText.toLowerCase() : rowText; + const matchIndex = textToSearch.indexOf(searchTerm); + if (matchIndex !== -1) { + const matchContext = this.getMatchContext(rowText, matchIndex, searchTerm.length); results.push({ file: `${filePath}:${sheetName}!Row${rowNumber}`, @@ -572,17 +566,13 @@ export interface SearchSessionOptions { ignoreCase: boolean, maxResults?: number, filePattern?: string, - literalSearch?: boolean + _literalSearch?: boolean ): Promise { const results: SearchResult[] = []; - // Build regex for matching content, with ReDoS protection - // When literalSearch is true, escape the pattern so it's matched literally - const flags = ignoreCase ? 'i' : ''; - const effectivePattern = literalSearch - ? pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') - : pattern; - const { regex } = buildSafeRegex(effectivePattern, flags); + // Office file search always uses literal matching to prevent ReDoS. + // Regex patterns are treated as literal strings — this is intentional. + const searchTerm = ignoreCase ? pattern.toLowerCase() : pattern; let docxFiles = await this.findDocxFiles(rootPath); @@ -593,8 +583,7 @@ export interface SearchSessionOptions { return patterns.some(pat => { if (pat.includes('*')) { const regexPat = pat.replace(/\./g, '\\.').replace(/\*/g, '.*'); - const { regex: globRegex } = buildSafeRegex(`^${regexPat}$`, 'i'); - return globRegex.test(fileName); + return new RegExp(`^${regexPat}$`, 'i').test(fileName); } return fileName.toLowerCase() === pat.toLowerCase(); }); @@ -630,11 +619,10 @@ export interface SearchSessionOptions { if (!text || !text.trim()) continue; lineNum++; - if (regex.test(text)) { - const match = text.match(regex); - const matchContext = match - ? this.getMatchContext(text, match.index || 0, match[0].length) - : text.substring(0, 150); + const textToSearch = ignoreCase ? text.toLowerCase() : text; + const matchIndex = textToSearch.indexOf(searchTerm); + if (matchIndex !== -1) { + const matchContext = this.getMatchContext(text, matchIndex, searchTerm.length); const partName = xmlPath === 'word/document.xml' ? '' : `:${xmlPath.replace('word/', '')}`; results.push({ From f376fffe310bd1c63038f9b02e19744f6b02a54d Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Fri, 3 Apr 2026 08:16:09 +0000 Subject: [PATCH 5/6] fix: escape all regex metacharacters in glob-to-regex conversion for Office file filtering The filePattern filter for Excel (xlsx) and DOCX content searches converted glob patterns to regex by only escaping '.' and '*'. Characters like '(', ')', '[', ']', '+', '^', '$', '{', '}' and '|' were left unescaped, causing incorrect filename matches for real-world patterns such as: - report(2024).xlsx -> '(' treated as regex group start - [draft].xlsx -> '[draft]' treated as character class - file+notes.docx -> '+' treated as quantifier Fix: escape all regex special characters first (excluding '*'), then convert the remaining glob '*' wildcards to '.*'. Both the Excel and DOCX filter paths are updated. Addresses CodeRabbit major finding on PR #400. --- src/search-manager.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/search-manager.ts b/src/search-manager.ts index 9fc8e5bb..5e03ba12 100644 --- a/src/search-manager.ts +++ b/src/search-manager.ts @@ -408,8 +408,13 @@ export interface SearchSessionOptions { return patterns.some(pat => { // Support glob-like patterns if (pat.includes('*')) { - // Glob patterns are safe (generated from sanitized file extensions, not user regex) - const regexPat = pat.replace(/\./g, '\\.').replace(/\*/g, '.*'); + // Escape all regex metacharacters first (preserving * for glob expansion), + // then convert the remaining * wildcards to .* for glob matching. + // Without this, patterns like report(2024).xlsx or [draft].xlsx would be + // misinterpreted as regex groups/character-classes. + const regexPat = pat + .replace(/[.+^${}()|[\]\\]/g, '\\$&') // escape metacharacters except * + .replace(/\*/g, '.*'); // glob * → regex .* return new RegExp(`^${regexPat}$`, 'i').test(fileName); } // Exact match (case-insensitive) @@ -582,7 +587,9 @@ export interface SearchSessionOptions { const fileName = path.basename(filePath); return patterns.some(pat => { if (pat.includes('*')) { - const regexPat = pat.replace(/\./g, '\\.').replace(/\*/g, '.*'); + const regexPat = pat + .replace(/[.+^${}()|[\]\\]/g, '\\$&') // escape metacharacters except * + .replace(/\*/g, '.*'); // glob * → regex .* return new RegExp(`^${regexPat}$`, 'i').test(fileName); } return fileName.toLowerCase() === pat.toLowerCase(); From 1b25823babc829d7ee0f6f06f1049d1f141ba103 Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Wed, 8 Apr 2026 13:04:09 +0400 Subject: [PATCH 6/6] fix: remove unused isSafeRegex/buildSafeRegex per review feedback Co-Authored-By: Claude Opus 4.6 (1M context) --- src/search-manager.ts | 45 ------------------------------------------- 1 file changed, 45 deletions(-) diff --git a/src/search-manager.ts b/src/search-manager.ts index 5e03ba12..d5b72fd4 100644 --- a/src/search-manager.ts +++ b/src/search-manager.ts @@ -7,51 +7,6 @@ import { getRipgrepPath } from './utils/ripgrep-resolver.js'; import { isExcelFile } from './utils/files/index.js'; import PizZip from 'pizzip'; -/** - * Check if a regex pattern is safe from catastrophic backtracking (ReDoS). - * Rejects patterns with nested quantifiers like (a+)+, (a*)+, (a+)*, (a*)*, - * and similar constructs that cause exponential runtime. - */ -export function isSafeRegex(pattern: string): boolean { - // Detect nested quantifiers: a group containing a quantifier, followed by a quantifier - // Matches patterns like (a+)+, (a+)*, (a*)+, (a*)*, (?:a+)+, etc. - // Also catches {n,m} style quantifiers nested inside quantified groups - const nestedQuantifier = /([^\\]|^)\((?:[^)]*[+*}])\s*\)[+*?]|\((?:[^)]*[+*}])\s*\)\{/; - if (nestedQuantifier.test(pattern)) { - return false; - } - - // Detect overlapping alternations in quantified groups: (a|a)+, (a|aa)+, (\w|\d)+ - // These can cause catastrophic backtracking even without a second outer quantifier - const overlappingAlt = /\((?:[^)]*\|[^)]*)\)[+*]/; - if (overlappingAlt.test(pattern)) { - return false; - } - - return true; -} - -/** - * Build a RegExp safely, falling back to literal string matching if the pattern - * is invalid or vulnerable to ReDoS. - * Returns { regex, isLiteral } so callers know if fallback occurred. - */ -export function buildSafeRegex(pattern: string, flags: string): { regex: RegExp; isLiteral: boolean } { - // Check for ReDoS-prone patterns first - if (!isSafeRegex(pattern)) { - const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - return { regex: new RegExp(escaped, flags), isLiteral: true }; - } - - try { - return { regex: new RegExp(pattern, flags), isLiteral: false }; - } catch { - // If pattern is not valid regex, escape it for literal matching - const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - return { regex: new RegExp(escaped, flags), isLiteral: true }; - } -} - export interface SearchResult { file: string; line?: number;