feat: harden scope writes, iteration, and readSize (#898)

Block writes to dangerous keys in assign/capture/increment/decrement, use own-property Symbol.iterator for plain objects when ownPropertyOnly is true, fix inherited size reads, and sanitize filter iteration scopes.

Co-authored-by: Cursor <[email protected]>

docs/source/tutorials/options.md

@@ -138,9 +138,7 @@ It defaults to `false`. For example, when set to `true`, a blank string would ev
 
 **lenientIf** modifies the behavior of `strictVariables` to allow handling optional variables. If set to `true`, an undefined variable will *not* cause an exception in the following two situations: a) it is the condition to an `if`, `elsif`, or `unless` tag; b) it occurs right before a `default` filter. Irrelevant if `strictVariables` is not set. Defaults to `false`.
 
-**ownPropertyOnly** hides scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates. Defaults to `true`.
-
-Built-in DoS limits and host isolation guidance are documented in [Security Model](./security-model.html).
+**ownPropertyOnly** limits template property reads on plain scope objects to own properties (no inherited prototype keys). Defaults to `true`. Blocked keys (`__proto__`, `constructor`, `prototype`) always apply. [`Drop`][drop] values, iteration, `.size`/`.first`/`.last`, filters, and custom tags follow separate rules—see [Security Model](./security-model.html).
 
 {% note info Nonexistent Tags %}
 Nonexistent tags always throw errors during parsing and this behavior cannot be customized.
@@ -163,3 +161,4 @@ Parameter orders are ignored by default, for example `{% for i in (1..8) reverse
 [jekyllInclude]: /api/interfaces/LiquidOptions.html#jekyllInclude
 [raw]: ../filters/raw.html
 [outputEscape]: /api/interfaces/LiquidOptions.html#outputEscape
+[drop]: /api/classes/Drop.html

docs/source/tutorials/security-model.md

@@ -50,9 +50,18 @@ The `memoryLimit` option was removed in v11; enforce memory limits at the host o
 
 ## `ownPropertyOnly` and scope data
 
-With [`ownPropertyOnly`][ownPropertyOnly] `true`, plain scope objects only expose **own** properties (no inherited / `Object.prototype` keys). Default `true`. Use `true` for untrusted or polluted objects; add [`strictVariables`][strictVariables] if missing paths should error. Override per render via [`RenderOptions`][renderOwnPropertyOnly]. This is a read policy for scope data—not a sandbox for filters, tags, or your code.
+[`ownPropertyOnly`][ownPropertyOnly] controls **template property reads on plain scope objects** (objects whose prototype is `null` or `Object.prototype`). Default `true`. When enabled, only own enumerable properties are visible to variable lookup; inherited keys from `Object.prototype` or other prototypes are hidden.
 
-LiquidJS also blocks template access to the property names `__proto__`, `constructor`, and `prototype` at any depth, and omits those keys when building null-prototype managed scopes (for example loop and `{% render %}` locals). For deeply untrusted input, pre-sanitize scope objects before passing them to `render()` (for example with [@hapi/bourne](https://www.npmjs.com/package/@hapi/bourne)).
+**Always blocked** (regardless of `ownPropertyOnly`): template access to the property names `__proto__`, `constructor`, and `prototype`, and writes to those names via `{% assign %}`, `{% capture %}`, `{% increment %}`, and `{% decrement %}`. Managed scopes built with null prototypes (loop locals, `{% render %}` bindings, filter iteration scopes) omit those keys when created from user data.
+
+**Exceptions** — `ownPropertyOnly` does not restrict:
+
+- [`Drop`][drop] values: prototype chain and [`liquidMethodMissing`][liquidMethodMissing] still apply; audit custom drops like privileged code.
+- Iteration (`{% for %}`, `{% tablerow %}`, `{% render for %}`): class instances and drops keep their iterators; plain objects only iterate via an own `Symbol.iterator`.
+- Liquid pseudo-properties `.size`, `.first`, and `.last`: arrays and strings use length/index rules; `Map`/`Set` use their native size; plain objects with an own `size` property use that value (inherited `size` on plain objects is ignored when `ownPropertyOnly` is `true`).
+- Filters and custom tags: operate on resolved values with their own semantics.
+
+Use `true` for untrusted or polluted objects; add [`strictVariables`][strictVariables] if missing paths should error. Override per render via [`RenderOptions`][renderOwnPropertyOnly]. For deeply untrusted input, pre-sanitize scope objects before `render()` (for example with [@hapi/bourne](https://www.npmjs.com/package/@hapi/bourne)). This is a read policy for scope data—not a sandbox for filters, tags, or your code.
 
 ## Custom `Drop` classes
 

src/context/context.ts

@@ -139,7 +139,7 @@ export class Context {
     const value = readJSProperty(obj, key, this.ownPropertyOnly)
     if (value === undefined && obj instanceof Drop) return obj.liquidMethodMissing(key, this)
     if (isFunction(value)) return value.call(obj)
-    if (key === 'size') return readSize(obj)
+    if (key === 'size') return readSize(obj, this.ownPropertyOnly)
     else if (key === 'first') return readFirst(obj, this.ownPropertyOnly)
     else if (key === 'last') return readLast(obj, this.ownPropertyOnly)
     return value
@@ -162,8 +162,10 @@ function readLast (obj: Scope, ownPropertyOnly: boolean) {
   return readJSProperty(obj, 'last', ownPropertyOnly)
 }
 
-function readSize (obj: Scope) {
-  if (hasOwnProperty.call(obj, 'size') || obj['size'] !== undefined) return obj['size']
+function readSize (obj: Scope, ownPropertyOnly: boolean) {
+  if (hasOwnProperty.call(obj, 'size')) return obj['size']
+  if (!ownPropertyOnly && obj['size'] !== undefined) return obj['size']
   if (isArray(obj) || isString(obj)) return obj.length
+  if (obj instanceof Map || obj instanceof Set) return obj.size
   if (typeof obj === 'object') return Object.keys(obj).length
 }

src/filters/array.ts

@@ -3,6 +3,7 @@ import { arrayIncludes, equals, evalToken, isTruthy } from '../render'
 import { Value, FilterImpl } from '../template'
 import { Tokenizer } from '../parser'
 import type { Scope } from '../context'
+import { createScope } from '../context/scope'
 import { EmptyDrop } from '../drop'
 
 export const join = argumentsToValue(function (this: FilterImpl, v: any[], arg: string) {
@@ -134,7 +135,7 @@ function * filter_exp<T extends object> (this: FilterImpl, include: boolean, arr
   const keyTemplate = new Value(stringify(exp), this.liquid)
   const array = toArray(arr)
   for (const item of array) {
-    this.context.push({ [itemName]: item })
+    this.context.push(createScope({ [itemName]: item }))
     const value = yield keyTemplate.value(this.context)
     this.context.pop()
     if (value === include) filtered.push(item)
@@ -160,7 +161,7 @@ export function * reject_exp<T extends object> (this: FilterImpl, arr: T[], item
 
 export function * group_by<T extends object> (this: FilterImpl, arr: T[], property: string): IterableIterator<unknown> {
   const map = new Map()
-  arr = toEnumerable(arr)
+  arr = toEnumerable(arr, this.context.ownPropertyOnly)
   const token = new Tokenizer(stringify(property)).readScopeValue()
   for (const item of arr) {
     const key = yield evalToken(token, this.context.spawn(item))
@@ -173,9 +174,9 @@ export function * group_by<T extends object> (this: FilterImpl, arr: T[], proper
 export function * group_by_exp<T extends object> (this: FilterImpl, arr: T[], itemName: string, exp: string): IterableIterator<unknown> {
   const map = new Map()
   const keyTemplate = new Value(stringify(exp), this.liquid)
-  arr = toEnumerable(arr)
+  arr = toEnumerable(arr, this.context.ownPropertyOnly)
   for (const item of arr) {
-    this.context.push({ [itemName]: item })
+    this.context.push(createScope({ [itemName]: item }))
     const key = yield keyTemplate.value(this.context)
     this.context.pop()
     if (!map.has(key)) map.set(key, [])

src/tags/assign.ts

@@ -1,4 +1,5 @@
 import { Value, Liquid, TopLevelToken, TagToken, Context, Tag } from '..'
+import { isBlockedScopeKey } from '../context/scope'
 import { Arguments } from '../template'
 import { IdentifierToken } from '../tokens'
 
@@ -20,6 +21,7 @@ export default class extends Tag {
     this.value = new Value(this.tokenizer.readFilteredValue(), this.liquid)
   }
   * render (ctx: Context): Generator<unknown, void, unknown> {
+    if (isBlockedScopeKey(this.key)) return
     ctx.bottom()[this.key] = yield this.value.value(ctx, this.liquid.options.lenientIf)
   }
 

src/tags/capture.ts

@@ -1,4 +1,5 @@
 import { Liquid, Tag, Template, Context, TagToken, TopLevelToken } from '..'
+import { isBlockedScopeKey } from '../context/scope'
 import { Parser } from '../parser'
 import { IdentifierToken, QuotedToken } from '../tokens'
 import { isTagToken } from '../util'
@@ -31,6 +32,7 @@ export default class extends Tag {
   * render (ctx: Context): Generator<unknown, void, string> {
     const r = this.liquid.renderer
     const html = yield r.renderTemplates(this.templates, ctx)
+    if (isBlockedScopeKey(this.variable)) return
     ctx.bottom()[this.variable] = html
   }
 

src/tags/decrement.ts

@@ -1,4 +1,5 @@
 import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..'
+import { isBlockedScopeKey } from '../context/scope'
 import { IdentifierToken } from '../tokens'
 import { isNumber, stringify } from '../util'
 
@@ -11,6 +12,7 @@ export default class extends Tag {
     this.variable = this.identifier.content
   }
   render (context: Context, emitter: Emitter) {
+    if (isBlockedScopeKey(this.variable)) return
     const scope = context.environments
     if (!isNumber(scope[this.variable])) {
       scope[this.variable] = 0

src/tags/for.ts

@@ -52,7 +52,7 @@ export default class extends Tag {
       ? Object.keys(hash).filter(x => MODIFIERS.includes(x))
       : MODIFIERS.filter(x => hash[x] !== undefined)
 
-    let collection = toEnumerable(yield evalToken(this.collection, ctx))
+    let collection = toEnumerable(yield evalToken(this.collection, ctx), ctx.ownPropertyOnly)
     collection = modifiers.reduce((collection, modifier: valueOf<typeof MODIFIERS>) => {
       if (modifier === 'offset') return offset(collection, hash['offset'])
       if (modifier === 'limit') return limit(collection, hash['limit'])

src/tags/increment.ts

@@ -1,4 +1,5 @@
 import { isNumber, stringify } from '../util'
+import { isBlockedScopeKey } from '../context/scope'
 import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..'
 import { IdentifierToken } from '../tokens'
 
@@ -11,6 +12,7 @@ export default class extends Tag {
     this.variable = this.identifier.content
   }
   render (context: Context, emitter: Emitter) {
+    if (isBlockedScopeKey(this.variable)) return
     const scope = context.environments
     if (!isNumber(scope[this.variable])) {
       scope[this.variable] = 0

src/tags/render.ts

@@ -70,7 +70,7 @@ export default class extends Tag {
 
     if (this.forBinding) {
       const { value, alias } = this.forBinding
-      const collection = toEnumerable(yield evalToken(value, ctx))
+      const collection = toEnumerable(yield evalToken(value, ctx), ctx.ownPropertyOnly)
       scope['forloop'] = new ForloopDrop(collection.length, value.getText(), alias as string)
       for (const item of collection) {
         scope[alias as string] = item

src/tags/tablerow.ts

@@ -39,7 +39,7 @@ export default class extends Tag {
   }
 
   * render (ctx: Context, emitter: Emitter): Generator<unknown, void, unknown> {
-    let collection = toEnumerable(yield evalToken(this.collection, ctx))
+    let collection = toEnumerable(yield evalToken(this.collection, ctx), ctx.ownPropertyOnly)
     const args = (yield this.args.render(ctx)) as Record<string, any>
     const offset = args.offset || 0
     const limit = (args.limit === undefined) ? collection.length : args.limit

src/util/underscore.ts

@@ -47,11 +47,11 @@ export function readArrayElement (arr: any[], index: number, ownPropertyOnly: bo
   return arr[index]
 }
 
-export function toEnumerable<T = unknown> (val: any): T[] {
+export function toEnumerable<T = unknown> (val: any, ownPropertyOnly = false): T[] {
   val = toValue(val)
   if (isArray(val)) return val
   if (isString(val) && val.length > 0) return [val] as unknown as T[]
-  if (isIterable(val)) return Array.from(val)
+  if (isIterable(val, ownPropertyOnly)) return Array.from(val)
   if (isObject(val)) return Object.keys(val).map((key) => [key, val[key]]) as unknown as T[]
   return []
 }
@@ -96,8 +96,17 @@ export function isArrayLike (value: any): value is any[] {
   return value && isNumber(value.length)
 }
 
-export function isIterable (value: any): value is Iterable<any> {
-  return isObject(value) && Symbol.iterator in value
+export function isIterable (value: any, ownPropertyOnly = false): value is Iterable<any> {
+  value = toValue(value)
+  if (!isObject(value)) return false
+  if (isArray(value)) return true
+  if (value instanceof Drop) return Symbol.iterator in value
+  if (ownPropertyOnly) {
+    const proto = Object.getPrototypeOf(value)
+    const isPlain = proto === null || proto === Object.prototype
+    if (isPlain) return hasOwnProperty.call(value, Symbol.iterator)
+  }
+  return Symbol.iterator in value
 }
 
 /*

test/integration/liquid/scope-security.spec.ts

@@ -1,4 +1,5 @@
 import { Liquid } from '../../../src/liquid'
+import { Drop } from '../../../src/drop/drop'
 
 describe('scope security', function () {
   let liquid: Liquid
@@ -55,4 +56,52 @@ describe('scope security', function () {
     const scope = { foo: { __proto__: { bar: 'BAR' } } }
     await expect(liquid.parseAndRender('{{ foo.__proto__.bar }}', scope, { ownPropertyOnly: false })).resolves.toBe('')
   })
+
+  it('should not write increment to __proto__ on user scope', async function () {
+    const scope = Object.create(null) as Record<string, unknown>
+    await expect(liquid.parseAndRender('{% increment __proto__ %}', scope)).resolves.toBe('')
+    expect(Object.prototype).toEqual(Object.prototype)
+    expect(scope).toEqual({})
+  })
+
+  it('should not write assign to __proto__ on user scope', async function () {
+    const scope = { safe: 'ok' }
+    await expect(liquid.parseAndRender(
+      '{% assign __proto__ = obj %}',
+      { ...scope, obj: { polluted: true } }
+    )).resolves.toBe('')
+    expect((Object.prototype as any).polluted).toBeUndefined()
+  })
+
+  it('should not iterate plain objects via inherited Symbol.iterator', async function () {
+    // eslint-disable-next-line no-extend-native
+    (Object.prototype as any)[Symbol.iterator] = function * () { yield 'inherited' }
+    try {
+      await expect(liquid.parseAndRender(
+        '{% for x in obj %}{{ x }}{% endfor %}',
+        { obj: {} }
+      )).resolves.toBe('')
+    } finally {
+      delete (Object.prototype as any)[Symbol.iterator]
+    }
+  })
+
+  it('should not read inherited size on plain objects', async function () {
+    const obj = Object.create({ size: 99 })
+    obj.own = 'yes'
+    await expect(liquid.parseAndRender('{{ obj.size }}', { obj })).resolves.toBe('1')
+  })
+
+  it('should still iterate Drop with Symbol.iterator', async function () {
+    class IterableDrop extends Drop {
+      * [Symbol.iterator] () {
+        yield 'a'
+        yield 'b'
+      }
+    }
+    await expect(liquid.parseAndRender(
+      '{% for x in drop %}{{ x }}{% endfor %}',
+      { drop: new IterableDrop() }
+    )).resolves.toBe('ab')
+  })
 })