From 7ab49f999ac045ec1e87f3a7a9fd68dd9e8602b3 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Mon, 6 Jul 2026 23:54:00 +0800 Subject: [PATCH] fix: charge join/json/inspect filters by produced output size (#925) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(filters): charge join/array_to_sentence_string by output size join charged memoryLimit by array element count, not by the string it produces, letting concat doubling (cheap reference copies) inflate an array's element count and then materialize a huge string via join far past the configured memoryLimit (GHSA-4r6h-5v86-94p3). Charge by the sum of stringified element lengths plus separators before allocating. Apply the same fix to the sibling array_to_sentence_string filter. Co-authored-by: Cursor * refactor(filters): simplify join output-size accounting Sum stringified element lengths in a single pass and keep the guarded Array.prototype.join for the result, instead of building an intermediate parts array. Co-authored-by: Cursor * fix(filters): charge json/jsonify/inspect serialization to memoryLimit json/jsonify/inspect serialized values without charging memoryLimit, so a concat-doubled array (cheap reference copies) could be materialized into a huge JSON string past the configured limit — the same unbounded class as the join bug (GHSA-4r6h-5v86-94p3). Charge via a JSON.stringify replacer that accounts string lengths as it walks, aborting mid- serialization instead of allocating the full blob first. Co-authored-by: Cursor * fix(memory): charge rendered output to memoryLimit at emission Move output-length accounting into the emitters, which charge each written chunk against ctx.memoryLimit right before it reaches the result string or stream. Filters/tags now only pre-charge the extra working memory they allocate apart from that output, so join drops its bespoke output-size counting and charges array.length like its siblings. The block.super capture emitter intentionally omits the limiter to avoid double-counting content that is re-emitted through the final emitter. Co-authored-by: Cursor * refactor(filters): rely on emitter output charge for json/inspect/array_to_sentence_string With rendered output charged at emission, these filters no longer need bespoke output-size counting: the emitted case is covered by the final emitter. Revert json/inspect to their original form and array_to_sentence_string to its element-count charge, dropping the non-emitted `| size` guards. Co-authored-by: Cursor * revert(memory): drop emitter output charge, restore filter output-size accounting join/array_to_sentence_string/json/inspect charge memoryLimit by the string they materialize (not element count), so discarded results like {% assign out = a | join %}{{ out | size }} are still bounded. Remove the emitter-level limiter added in 2f343f063; it cannot catch materialized-but-not-emitted values. Co-authored-by: Cursor * fix(filters): charge json/inspect replacer by serialized node size Replace the flat 1-unit charge for non-string JSON nodes with per-type estimates (primitives via JSON.stringify length, containers by structure). Co-authored-by: Cursor --------- Co-authored-by: Cursor --- src/filters/array.ts | 5 ++-- src/filters/misc.ts | 34 ++++++++++++++++++++---- src/filters/string.ts | 4 ++- test/integration/liquid/dos.spec.ts | 41 +++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/filters/array.ts b/src/filters/array.ts index e714d6588..57a7d802d 100644 --- a/src/filters/array.ts +++ b/src/filters/array.ts @@ -8,8 +8,9 @@ import { EmptyDrop } from '../drop' export const join = argumentsToValue(function (this: FilterImpl, v: any[], arg: string) { const array = toArray(v) const sep = isNil(arg) ? ' ' : stringify(arg) - const complexity = array.length * (1 + sep.length) - this.context.memoryLimit.use(complexity) + let outputSize = sep.length * Math.max(array.length - 1, 0) + for (let i = 0; i < array.length; i++) outputSize += String(array[i]).length + this.context.memoryLimit.use(outputSize) return Array.prototype.join.call(array, sep) }) export const last = argumentsToValue(function (this: FilterImpl, v: any) { diff --git a/src/filters/misc.ts b/src/filters/misc.ts index 57ed57812..447376550 100644 --- a/src/filters/misc.ts +++ b/src/filters/misc.ts @@ -2,6 +2,18 @@ import { isFalsy } from '../render/boolean' import { identify, isArray, isString, toValue } from '../util/underscore' import { FilterImpl } from '../template' +function chargeJsonReplacerValue (memoryLimit: { use(count: number): void }, val: unknown) { + if (typeof val === 'string') { + memoryLimit.use(val.length) + } else if (val === null || typeof val === 'number' || typeof val === 'boolean') { + memoryLimit.use(JSON.stringify(val).length) + } else if (Array.isArray(val)) { + memoryLimit.use(val.length + 1) + } else if (typeof val === 'object') { + memoryLimit.use(2) + } +} + function defaultFilter (this: FilterImpl, value: T1, defaultValue: T2, ...args: Array<[string, any]>): T1 | T2 { value = toValue(value) if (isArray(value) || isString(value)) return value.length ? value : defaultValue @@ -9,18 +21,30 @@ function defaultFilter (this: FilterImpl, value: T1, def return isFalsy(value, this.context) ? defaultValue : value } -function json (value: any, space = 0) { - return JSON.stringify(value, null, space) +function json (this: FilterImpl, value: any, space = 0) { + const memoryLimit = this.context.memoryLimit + return JSON.stringify(value, (_key, val) => { + chargeJsonReplacerValue(memoryLimit, val) + return val + }, space) } -function inspect (value: any, space = 0) { +function inspect (this: FilterImpl, value: any, space = 0) { + const memoryLimit = this.context.memoryLimit const ancestors: object[] = [] return JSON.stringify(value, function (this: unknown, _key: unknown, value: any) { - if (typeof value !== 'object' || value === null) return value + if (typeof value !== 'object' || value === null) { + chargeJsonReplacerValue(memoryLimit, value) + return value + } // `this` is the object that value is contained in, i.e., its direct parent. while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) ancestors.pop() - if (ancestors.includes(value)) return '[Circular]' + if (ancestors.includes(value)) { + memoryLimit.use('[Circular]'.length) + return '[Circular]' + } ancestors.push(value) + chargeJsonReplacerValue(memoryLimit, value) return value }, space) } diff --git a/src/filters/string.ts b/src/filters/string.ts index f96d4b492..d0c59708e 100644 --- a/src/filters/string.ts +++ b/src/filters/string.ts @@ -209,7 +209,9 @@ export function number_of_words (this: FilterImpl, input: string, mode?: 'cjk' | export function array_to_sentence_string (this: FilterImpl, array: unknown[], connector = 'and') { connector = stringify(connector) - this.context.memoryLimit.use(array.length + connector.length) + let outputSize = connector.length + array.length * 2 + for (let i = 0; i < array.length; i++) outputSize += stringify(array[i]).length + this.context.memoryLimit.use(outputSize) switch (array.length) { case 0: return '' diff --git a/test/integration/liquid/dos.spec.ts b/test/integration/liquid/dos.spec.ts index 2aea12c48..65d78adb5 100644 --- a/test/integration/liquid/dos.spec.ts +++ b/test/integration/liquid/dos.spec.ts @@ -89,6 +89,47 @@ describe('DoS related', function () { const liquid = new Liquid({ memoryLimit: 100 }) await expect(liquid.parseAndRender('{{ array | sample: 1 | size }}', { array })).rejects.toThrow('memory alloc limit exceeded') }) + it('should charge join by produced output size, not element count', () => { + const array = ['a'.repeat(100), 'b'.repeat(100)] + const liquid = new Liquid({ memoryLimit: 100 }) + expect(() => liquid.parseAndRenderSync('{{ array | join: "" }}', { array })) + .toThrow('memory alloc limit exceeded') + }) + it('should allow join within memoryLimit', () => { + const array = ['a'.repeat(20), 'b'.repeat(20)] + const liquid = new Liquid({ memoryLimit: 100 }) + expect(liquid.parseAndRenderSync('{{ array | join: "" }}', { array })).toBe('a'.repeat(20) + 'b'.repeat(20)) + }) + it('should prevent concat doubling from bypassing join memoryLimit', () => { + const liquid = new Liquid({ memoryLimit: 1e4 }) + const src = '{%- assign a = s | split: "NOSEP" -%}' + + '{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}' + + '{{ a | join: "" | size }}' + expect(() => liquid.parseAndRenderSync(src, { s: 'a'.repeat(5000) })) + .toThrow('memory alloc limit exceeded') + }) + it('should charge array_to_sentence_string by produced output size', () => { + const array = ['a'.repeat(100), 'b'.repeat(100), 'c'.repeat(100)] + const liquid = new Liquid({ memoryLimit: 100 }) + expect(() => liquid.parseAndRenderSync('{{ array | array_to_sentence_string }}', { array })) + .toThrow('memory alloc limit exceeded') + }) + it('should charge json serialization of concat-doubled arrays', () => { + const liquid = new Liquid({ memoryLimit: 1e4 }) + const src = '{%- assign a = s | split: "NOSEP" -%}' + + '{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}' + + '{{ a | json | size }}' + expect(() => liquid.parseAndRenderSync(src, { s: 'a'.repeat(5000) })) + .toThrow('memory alloc limit exceeded') + }) + it('should charge inspect serialization of concat-doubled arrays', () => { + const liquid = new Liquid({ memoryLimit: 1e4 }) + const src = '{%- assign a = s | split: "NOSEP" -%}' + + '{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}' + + '{{ a | inspect | size }}' + expect(() => liquid.parseAndRenderSync(src, { s: 'a'.repeat(5000) })) + .toThrow('memory alloc limit exceeded') + }) 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) }))