From 706cb02d30cbb3ea2f428282ea7c19885248e7b3 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Sun, 14 Jun 2026 16:25:31 +0800 Subject: [PATCH] fix(security): charge json/inspect indentation and keys to memoryLimit The incremental per-node charge ignored two output contributors that can greatly exceed the charged amount: the `space` indentation (which scales with nesting depth) and object property keys. Both let `{{ data | json: 10 }}` or key-heavy objects produce far larger strings than memoryLimit accounts for. Charge a depth-scaled indentation cost and the key length per node, keeping the total a strict lower bound of the output length. Also trims the redundant narrating comments added with the original fix. Co-authored-by: Cursor --- src/filters/misc.ts | 51 ++++++++++++++++------------- test/integration/liquid/dos.spec.ts | 12 +++++++ 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/filters/misc.ts b/src/filters/misc.ts index 8530a9808..961a9146b 100644 --- a/src/filters/misc.ts +++ b/src/filters/misc.ts @@ -1,5 +1,6 @@ import { isFalsy } from '../render/boolean' import { identify, isArray, isString, toValue } from '../util/underscore' +import { Limiter } from '../util/limiter' import { FilterImpl } from '../template' function defaultFilter (this: FilterImpl, value: T1, defaultValue: T2, ...args: Array<[string, any]>): T1 | T2 { @@ -9,41 +10,45 @@ function defaultFilter (this: FilterImpl, value: T1, def return isFalsy(value, this.context) ? defaultValue : value } -// A strict lower bound on the bytes a single node contributes to JSON output, -// excluding its children (which are visited separately). Charging this per node -// during traversal lets `memoryLimit` abort before the full string is built, -// while never over-charging an in-budget input (total charged <= output.length). function jsonNodeSize (value: any): number { - if (value === null) return 4 // null + if (value === null) return 4 switch (typeof value) { - case 'string': return value.length + 2 // quotes; escapes only add more + case 'string': return value.length + 2 case 'number': return ('' + value).length case 'boolean': return value ? 4 : 5 - case 'object': return 2 // {} or [] braces; entries charged on their own visits - default: return 0 // undefined/function/symbol are omitted from output + case 'object': return 2 + default: return 0 } } -function json (this: FilterImpl, value: any, space = 0) { - const memoryLimit = this.context.memoryLimit - return JSON.stringify(value, (_key: string, value: any) => { - memoryLimit.use(jsonNodeSize(value)) - return value - }, space) +function indentWidth (space: number | string): number { + if (typeof space === 'string') return Math.min(space.length, 10) + const n = Math.floor(space) + return n > 0 ? Math.min(n, 10) : 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) { - memoryLimit.use(jsonNodeSize(value)) - if (typeof value !== 'object' || value === null) return value - // `this` is the object that value is contained in, i.e., its direct parent. +function jsonReplacer (memoryLimit: Limiter, width: number, detectCircular: boolean) { + const ancestors: unknown[] = [] + return function (this: unknown, key: string, value: any) { + // `this` is the parent holding `value`; popping ancestors back to it yields the + // nesting depth, so we can charge a lower bound of the bytes `value` adds to the + // output (its own content, its key, and the indentation of its line). while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) ancestors.pop() - if (ancestors.includes(value)) return '[Circular]' + const keySize = isArray(this) ? 0 : key.length + memoryLimit.use(jsonNodeSize(value) + keySize + ancestors.length * width) + if (value === null || typeof value !== 'object') return value + if (detectCircular && ancestors.includes(value)) return '[Circular]' ancestors.push(value) return value - }, space) + } +} + +function json (this: FilterImpl, value: any, space: number | string = 0) { + return JSON.stringify(value, jsonReplacer(this.context.memoryLimit, indentWidth(space), false), space) +} + +function inspect (this: FilterImpl, value: any, space: number | string = 0) { + return JSON.stringify(value, jsonReplacer(this.context.memoryLimit, indentWidth(space), true), space) } function to_integer (value: any) { diff --git a/test/integration/liquid/dos.spec.ts b/test/integration/liquid/dos.spec.ts index 277b37f38..c99382472 100644 --- a/test/integration/liquid/dos.spec.ts +++ b/test/integration/liquid/dos.spec.ts @@ -104,6 +104,18 @@ describe('DoS related', function () { const liquid = new Liquid({ memoryLimit: 100 }) await expect(liquid.parseAndRender('{{ data | inspect }}', { data })).rejects.toThrow('memory alloc limit exceeded') }) + it('should charge json indentation (space) to memoryLimit', async () => { + const data = Array(50).fill(0) + const liquid = new Liquid({ memoryLimit: 200 }) + await expect(liquid.parseAndRender('{{ data | json }}', { data })).resolves.toBe('[' + Array(50).fill(0).join(',') + ']') + await expect(liquid.parseAndRender('{{ data | json: 10 }}', { data })).rejects.toThrow('memory alloc limit exceeded') + }) + it('should charge json object keys to memoryLimit', async () => { + const data: Record = {} + for (let i = 0; i < 20; i++) data['k' + i + 'x'.repeat(50)] = 0 + const liquid = new Liquid({ memoryLimit: 100 }) + await expect(liquid.parseAndRender('{{ data | json }}', { data })).rejects.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) }))