diff --git a/src/main/files/index.js b/src/main/files/index.js index 3896992..79f7d77 100644 --- a/src/main/files/index.js +++ b/src/main/files/index.js @@ -134,7 +134,12 @@ function register({ // search-in-files ipcMain.handle('search-in-files', async (_event, payload) => { - return searchInFiles(payload || {}); + const validation = validatePath(payload?.rootPath); + if (!validation.valid || !isPathAccessible(validation.resolved)) { + console.error('[SECURITY] Invalid search rootPath:', validation.error); + return []; + } + return searchInFiles({ ...(payload || {}), rootPath: validation.resolved }); }); // open-file-path diff --git a/src/main/files/search-in-files.js b/src/main/files/search-in-files.js index f95150d..c933fff 100644 --- a/src/main/files/search-in-files.js +++ b/src/main/files/search-in-files.js @@ -5,27 +5,61 @@ const path = require('path'); const MAX_RESULTS = 1000; const MAX_FILE_BYTES = 2 * 1024 * 1024; +const MAX_FILES = 10000; +const MAX_QUERY_LENGTH = 1024; const SKIP_DIRS = new Set(['node_modules', '.git', 'dist', '.next', '.cache']); +// Reject regexes with nested quantifiers / classic ReDoS shapes. +const UNSAFE_REGEX = /(\([^)]*[+*][^)]*\)[+*])|(\[[^\]]*\][+*])|(\.[+*]\.[+*])|(\(\?\:)/; + function listFiles(rootPath) { const out = []; const stack = [rootPath]; - while (stack.length) { + const visited = new Set(); + while (stack.length && out.length < MAX_FILES) { const dir = stack.pop(); + // Resolve symlinks and verify we haven't escaped the root. + let real; + try { + real = fs.realpathSync(dir); + } catch (_) { + continue; + } + if (visited.has(real)) continue; + visited.add(real); + const rootReal = fs.realpathSync(rootPath); + if (!real.startsWith(rootReal + path.sep) && real !== rootReal) continue; + let entries; try { - entries = fs.readdirSync(dir, { withFileTypes: true }); + entries = fs.readdirSync(real, { withFileTypes: true }); } catch (_) { continue; } for (const e of entries) { if (e.name.startsWith('.')) continue; - const full = path.join(dir, e.name); - if (e.isDirectory()) { - if (SKIP_DIRS.has(e.name)) continue; - stack.push(full); - } else if (e.isFile()) { - out.push(full); + const full = path.join(real, e.name); + // Follow symlinks but verify containment again. + try { + const s = fs.lstatSync(full); + if (s.isSymbolicLink()) { + const linkTarget = fs.realpathSync(full); + if (!linkTarget.startsWith(rootReal + path.sep) && linkTarget !== rootReal) continue; + } + } catch (_) { + continue; + } + try { + const s = fs.statSync(full); + if (s.isDirectory()) { + if (SKIP_DIRS.has(e.name)) continue; + stack.push(full); + } else if (s.isFile()) { + out.push(full); + if (out.length >= MAX_FILES) break; + } + } catch (_) { + continue; } } } @@ -34,6 +68,7 @@ function listFiles(rootPath) { function makeMatcher(query, isRegex, caseSensitive) { if (isRegex) { + if (UNSAFE_REGEX.test(query)) return null; try { const re = new RegExp(query, caseSensitive ? '' : 'i'); return (line) => re.test(line); @@ -50,11 +85,15 @@ function makeMatcher(query, isRegex, caseSensitive) { * Returns up to MAX_RESULTS matches of the form * { filePath, line, content }. * - * Skips binary files (> MAX_FILE_BYTES) and noisy directories - * (node_modules, .git, dist, .next, .cache) to keep latency low. + * Hardened against: + * - Path traversal via symlinks (verified after realpathSync) + * - ReDoS via nested-quantifier regex (rejected at match time) + * - Resource exhaustion via MAX_FILES + MAX_FILE_BYTES + MAX_RESULTS + * - Empty / oversized queries via MAX_QUERY_LENGTH */ function searchInFiles({ rootPath, query, isRegex = false, caseSensitive = false }) { if (!rootPath || !query) return []; + if (typeof query !== 'string' || query.length > MAX_QUERY_LENGTH) return []; const matcher = makeMatcher(query, isRegex, caseSensitive); if (!matcher) return []; diff --git a/tests/unit/main/files/search-in-files.test.js b/tests/unit/main/files/search-in-files.test.js index 89064c7..62e5ed5 100644 --- a/tests/unit/main/files/search-in-files.test.js +++ b/tests/unit/main/files/search-in-files.test.js @@ -84,4 +84,50 @@ describe('searchInFiles', () => { const out = searchInFiles({ rootPath: tmpDir, query: 'match' }); expect(out.length).toBe(1000); }); + + test('rejects queries longer than 1024 chars (DoS guard)', () => { + const f = path.join(tmpDir, 'a.md'); + fs.writeFileSync(f, 'hello'); + const longQ = 'a'.repeat(2000); + expect(searchInFiles({ rootPath: tmpDir, query: longQ })).toEqual([]); + }); + + test('rejects regexes with nested quantifiers (ReDoS guard)', () => { + const f = path.join(tmpDir, 'a.md'); + fs.writeFileSync(f, 'aaaaaaaaaaaaaab'); + // Classic catastrophic backtracking pattern + expect( + searchInFiles({ rootPath: tmpDir, query: '(a+)+b', isRegex: true }) + ).toEqual([]); + // Allowed: non-nested alternation + expect( + searchInFiles({ rootPath: tmpDir, query: 'a+b', isRegex: true }).length + ).toBeGreaterThanOrEqual(0); + }); + + test('does not follow symlinks that escape rootPath', () => { + // Create a target outside tmpDir and a symlink inside pointing to it. + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'search-outside-')); + const targetFile = path.join(outside, 'secret.md'); + fs.writeFileSync(targetFile, 'should not be searched'); + try { + fs.symlinkSync(targetFile, path.join(tmpDir, 'evil-link')); + const insideFile = path.join(tmpDir, 'inside.md'); + fs.writeFileSync(insideFile, 'inside match'); + const out = searchInFiles({ rootPath: tmpDir, query: 'match' }); + expect(out.find((r) => r.filePath.includes('evil-link'))).toBeUndefined(); + expect(out.find((r) => r.filePath === insideFile)).toBeTruthy(); + } finally { + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + + test('caps files traversed at 10000', () => { + // Create many small files + for (let i = 0; i < 50; i++) { + fs.writeFileSync(path.join(tmpDir, `f${i}.md`), 'x'); + } + const out = searchInFiles({ rootPath: tmpDir, query: 'x' }); + expect(out.length).toBeLessThanOrEqual(1000); // capped at MAX_RESULTS too + }); });