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 <[email protected]>

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<T1 extends boolean, T2> (this: FilterImpl, value: T1, defaultValue: T2, ...args: Array<[string, any]>): T1 | T2 {
@@ -9,41 +10,45 @@ function defaultFilter<T1 extends boolean, T2> (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
+    case 'object': return 2
-    default: return 0 // undefined/function/symbol are omitted from output
+    default: return 0
   }
 }
 
-function json (this: FilterImpl, value: any, space = 0) {
+function indentWidth (space: number | string): number {
-  const memoryLimit = this.context.memoryLimit
+  if (typeof space === 'string') return Math.min(space.length, 10)
-  return JSON.stringify(value, (_key: string, value: any) => {
+  const n = Math.floor(space)
-    memoryLimit.use(jsonNodeSize(value))
+  return n > 0 ? Math.min(n, 10) : 0
-    return value
-  }, space)
 }
 
-function inspect (this: FilterImpl, value: any, space = 0) {
+function jsonReplacer (memoryLimit: Limiter, width: number, detectCircular: boolean) {
-  const memoryLimit = this.context.memoryLimit
+  const ancestors: unknown[] = []
-  const ancestors: object[] = []
+  return function (this: unknown, key: string, value: any) {
-  return JSON.stringify(value, function (this: unknown, _key: unknown, value: any) {
+    // `this` is the parent holding `value`; popping ancestors back to it yields the
-    memoryLimit.use(jsonNodeSize(value))
+    // nesting depth, so we can charge a lower bound of the bytes `value` adds to the
-    if (typeof value !== 'object' || value === null) return value
+    // output (its own content, its key, and the indentation of its line).
-    // `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]'
+    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) {

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<string, number> = {}
+      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) }))