From 3616a744b9abeb425c217b340a2397d46176afb8 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Mon, 11 May 2026 23:59:40 +0800 Subject: [PATCH] fix(strip_html): rewrite as linear single-pass scan to avoid ReDoS (#896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(strip_html): rewrite as linear single-pass scan to avoid ReDoS The previous strip_html regex /||<[\s\S]*?>|/g contains lazy alternatives that backtrack O(n^2) on inputs with many unclosed `` and skip the whole block; cache "no closer after pos k" so subsequent unclosed ``. - otherwise treat as a generic `<...>` tag (matches the original behavior, where the `<[\s\S]*?>` alternative also caught comments). - if no closing `>` exists, emit the tail as literal text and stop. Total work is O(n). All existing strip_html test cases pass unchanged. Add regression tests covering the PoCs (`foo` repeats with `>` but no ``) plus a memoryLimit assertion. Co-authored-by: Cursor * refactor(strip_html): factor block kinds into a small table Same algorithm and complexity, fewer lines. Document why a regex-only solution can't be O(n) in V8 (no atomic groups / possessive quantifiers / memoization, so unrolled-loop patterns are still O(n^2) on unclosed openers — empirically confirmed: original 280KB ~4s, Friedl unrolled ~14s, atomic lookahead ~7s; tokenizer ~1ms). Co-authored-by: Cursor * refactor(strip_html): inline block kinds to match file style Drop the module-level STRIP_BLOCKS table; the rest of the file keeps each filter self-contained (only escapeMap/unescapeMap are top-level maps shared across filters). Two openers don't justify a table. Co-authored-by: Cursor * refactor(strip_html): unify raw-text blocks; treat as opaque In HTML5, '], [''], [''], ['<', '>']]) + let out = '' + let i = 0 + while (i < str.length) { + const lt = str.indexOf('<', i) + if (lt < 0) return out + str.slice(i) + out += str.slice(i, lt) + for (const [opener, closer] of blocks) { + if (!str.startsWith(opener, lt)) continue + const e = str.indexOf(closer, lt + opener.length) + if (e >= 0) { i = e + closer.length; break } + blocks.delete(opener) + } + if (i === lt) return out + str.slice(lt) + } + return out } diff --git a/test/integration/filters/html.spec.ts b/test/integration/filters/html.spec.ts index c4551dad4..e858420e0 100644 --- a/test/integration/filters/html.spec.ts +++ b/test/integration/filters/html.spec.ts @@ -57,6 +57,9 @@ describe('filters/html', function () { it('should strip multiline comments', function () { expect(liquid.parseAndRenderSync('{{""|strip_html}}')).toBe('') }) + it('should treat > inside comments as comment content (not a tag end)', function () { + expect(liquid.parseAndRenderSync('{{ "after" | strip_html }}')).toBe('after') + }) it('should strip all style tags and their contents', function () { return test('{{ "Ulysses?" | strip_html }}', 'Ulysses?') diff --git a/test/integration/liquid/dos.spec.ts b/test/integration/liquid/dos.spec.ts index 796f2db8e..85783564e 100644 --- a/test/integration/liquid/dos.spec.ts +++ b/test/integration/liquid/dos.spec.ts @@ -79,5 +79,30 @@ describe('DoS related', function () { await expect(liquid.parseAndRender(src, { array, count: 3 })).resolves.toBe('a a a a a a a a') await expect(liquid.parseAndRender(src, { array, count: 100 })).rejects.toThrow('memory alloc limit exceeded, line:1, col:26') }) + it('should charge strip_html input length to memoryLimit', () => { + const liquid = new Liquid({ memoryLimit: 100 }) + expect(() => liquid.parseAndRenderSync('{{ s | strip_html }}', { s: 'a'.repeat(200) })) + .toThrow('memory alloc limit exceeded') + }) + }) + describe('strip_html ReDoS', () => { + // Regression for O(n^2) backtracking on unclosed ` { + const liquid = new Liquid() + const payload = ' { + const liquid = new Liquid() + const payload = ' but no in linear time', () => { + const liquid = new Liquid() + const payload = '