fix: v11 scope security and ownPropertyOnly hardening (#898) (#938)

* feat: block dangerous scope keys and harden findScope (#898) Co-authored-by: Cursor <[email protected]> * docs: fix ownPropertyOnly default in security model Co-authored-by: Cursor <[email protected]> * 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]> * fix: tie proto key blocking to ownPropertyOnly policy Block __proto__, constructor, and prototype only when ownPropertyOnly is true or when access would traverse the prototype chain. Allow own properties with those names when ownPropertyOnly is false. Co-authored-by: Cursor <[email protected]> * fix: revert ownPropertyOnly iteration hardening Iteration is documented as an ownPropertyOnly exception; restore isIterable/toEnumerable and document inherited Symbol.iterator behavior. Co-authored-by: Cursor <[email protected]> * docs: fix ownPropertyOnly blocked-keys wording in options Co-authored-by: Cursor <[email protected]> * fix: unify blocked-key checks in findScope Use shouldBlockScopeKeyRead in findScope hasKey so inherited constructor/__proto__/prototype do not falsely match environments. Remove redundant globals hasKey check; globals remains the fallback scope. Co-authored-by: Cursor <[email protected]> * test: trim redundant scope-security integration tests Co-authored-by: Cursor <[email protected]> * refactor: move readSize to Context methods Move readSize, readFirst, and readLast to private Context methods using this.ownPropertyOnly. Remove redundant shouldBlockScopeKeyRead from findScope. Co-authored-by: Cursor <[email protected]> * refactor: wrap plain scopes in Context.push() Centralize null-prototype scope creation in push() so callers pass plain objects; Drop instances and existing null-proto frames are pushed as-is. Remove sanitizeScope in favor of createScope via Object.assign. * refactor: drop redundant tag write-path blocking Write blocking on assign/capture/increment/decrement duplicated read-side protection in readJSProperty; null-proto scopes from push already prevent prototype pollution on managed writes. Co-authored-by: Cursor <[email protected]> * fix: address scope-security review findings Restore null-prototype hardening for Jekyll include bindings, colocate blocked-key checks with readJSProperty, align ownPropertyOnly JSDoc with security docs, and drop integration tests duplicated in context.spec. Co-authored-by: Cursor <[email protected]> * refactor: simplify scope-security MR Drop null-prototype passthrough in push(), inline blocked-key checks, remove redundant createScope at include tag, trim verbose docs, and drop implementation-detail unit tests. Co-authored-by: Cursor <[email protected]> * refactor: trim scope-security helpers and docs Inline findScope and blocked-key checks, shorten ownPropertyOnly docs, and drop implementation-detail push() unit tests. Co-authored-by: Cursor <[email protected]> * refactor: encapsulate Drop passthrough in createScope * refactor: drop redundant typeof in blocked key check Set.has already returns false for non-string PropertyKey values; widen BLOCKED_SCOPE_KEYS type so TypeScript accepts the direct has(key) call. Co-authored-by: Cursor <[email protected]> * docs: shorten ownPropertyOnly proto-key wording Co-authored-by: Cursor <[email protected]> * fix: clarify blocked key checks in readJSProperty Split the OR condition into two explicit checks so inherited proto keys are always blocked and own proto keys are blocked only when ownPropertyOnly is true. Co-authored-by: Cursor <[email protected]> * fix: apply ownPropertyOnly uniformly in readJSProperty Proto keys block inherited access only; ownPropertyOnly is checked once before return for all keys. Own __proto__/constructor/prototype properties are readable—sanitize untrusted scope input. Co-authored-by: Cursor <[email protected]> * fix: remove BLOCKED_SCOPE_KEYS; ownPropertyOnly is the sole read policy Proto keys were incorrectly blocked even when ownPropertyOnly=false. Inherited access is now gated only by ownPropertyOnly; docs updated. Co-authored-by: Cursor <[email protected]> * fix: restore BLOCKED_SCOPE_KEYS gated by ownPropertyOnly Dangerous keys (__proto__, constructor, prototype) are blocked only when ownPropertyOnly is true (default). With false, full prototype access is allowed as an explicit opt-out; use bourne for untrusted input. Co-authored-by: Cursor <[email protected]> * docs: shorten ownPropertyOnly entry in options tutorial Details live in Security Model; keep options.md consistent with strictFilters/strictVariables tone. Co-authored-by: Cursor <[email protected]> * docs: simplify ownPropertyOnly JSDoc in LiquidOptions Co-authored-by: Cursor <[email protected]> * test: cover readSize branches in Context Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: Cursor <[email protected]>
2026-09-15 04:10:40 -07:00 · 2026-07-24 00:53:21 +08:00
parent 61ed163821
commit 964a63b362
12 changed files with 150 additions and 49 deletions
@@ -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. Defaults to `true`. See [Security Model](./security-model.html).

 {% note info Nonexistent Tags %}
 Nonexistent tags always throw errors during parsing and this behavior cannot be customized.
@@ -50,7 +50,11 @@ 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 `false` follows normal JS property access. 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.
+With [`ownPropertyOnly`][ownPropertyOnly] `true` (default), plain scope objects only expose **own** properties (no inherited / `Object.prototype` keys), and reads of `__proto__`, `constructor`, and `prototype` are blocked (own and inherited) as a prototype-pollution defense. With `false`, inherited properties and those keys are allowed—sanitize untrusted scope data (e.g. with [bourne](https://www.npmjs.com/package/bourne)) before passing it as scope. LiquidJS also uses null-prototype objects for managed scope frames (e.g. `{% capture %}`, `{% assign %}`) so internal frames do not inherit from `Object.prototype`.
+
+Not restricted: [`Drop`][drop] values, iteration via `Symbol.iterator`, `.size`/`.first`/`.last`, filters, and custom tags.
+
+Use `true` for untrusted 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.

 ## Custom `Drop` classes

@@ -58,6 +58,9 @@ describe('Context', function () {
    it('should return map size as size', async function () {
      expect(ctx.get(['map', 'size'])).toEqual(1)
    })
+    it('should return own size property', async function () {
+      expect(ctx.get(['zoo', 'size'])).toEqual(4)
+    })
    it('should return undefined if not have a size', async function () {
      expect(ctx.get(['one', 'size'])).toBeUndefined()
      expect(ctx.get(['non-exist', 'size'])).toBeUndefined()
@@ -130,6 +133,10 @@ describe('Context', function () {
      ctx = new Context({ foo: Object.create({ bar: 'BAR' }) }, { ownPropertyOnly: false } as any)
      return expect(ctx.getSync(['foo', 'bar'])).toEqual('BAR')
    })
+    it('should read inherited size when ownPropertyOnly=false', function () {
+      ctx = new Context({ foo: Object.create({ size: 99 }) }, { ownPropertyOnly: false } as any)
+      return expect(ctx.getSync(['foo', 'size'])).toEqual(99)
+    })
    it('renderOptions.ownPropertyOnly should override options.ownPropertyOnly', function () {
      ctx = new Context({ foo: Object.create({ bar: 'BAR' }) }, { ownPropertyOnly: false } as any, { ownPropertyOnly: true })
      return expect(ctx.getSync(['foo', 'bar'])).toEqual(undefined)
@@ -198,6 +205,36 @@ describe('Context', function () {
        delete (Array.prototype as any)[0]
      }
    })
+    it('should allow own blocked keys when ownPropertyOnly=false', function () {
+      ctx = new Context({
+        foo: {
+          ...JSON.parse('{"__proto__": {"bar": "BAR"}}'),
+          constructor: { name: 'Custom' },
+          prototype: { x: 1 }
+        }
+      }, { ownPropertyOnly: false } as any)
+      expect(ctx.getSync(['foo', '__proto__', 'bar'])).toEqual('BAR')
+      expect(ctx.getSync(['foo', 'constructor', 'name'])).toEqual('Custom')
+      expect(ctx.getSync(['foo', 'prototype', 'x'])).toEqual(1)
+    })
+    it('should allow inherited properties when ownPropertyOnly=false', function () {
+      ctx = new Context({ foo: Object.create({ __proto__: { bar: 'BAR' }, constructor: { name: 'Evil' } }) }, { ownPropertyOnly: false } as any)
+      expect(ctx.getSync(['foo', '__proto__', '__proto__', 'bar'])).toEqual('BAR')
+      expect(ctx.getSync(['foo', 'constructor', 'name'])).toEqual('Evil')
+    })
+    it('should block own constructor when ownPropertyOnly=true', function () {
+      ctx.push({ foo: { constructor: { name: 'Evil' } } })
+      expect(ctx.getSync(['foo', 'constructor'])).toEqual(undefined)
+    })
+    it('should block own prototype when ownPropertyOnly=true', function () {
+      ctx.push({ foo: { prototype: { bar: 'BAR' } } })
+      expect(ctx.getSync(['foo', 'prototype'])).toEqual(undefined)
+    })
+    it('should block own top-level __proto__ variable when ownPropertyOnly=true', function () {
+      ctx = new Context(JSON.parse('{"__proto__": {"bar": "BAR"}, "bar": "BAR"}'))
+      expect(ctx.getSync(['__proto__'])).toEqual(undefined)
+      expect(ctx.getSync(['bar'])).toEqual('BAR')
+    })
  })

  describe('.getAll()', function () {
@@ -221,6 +258,11 @@ describe('Context', function () {
      expect(ctx.getSync(['bar', 'foo'])).toEqual('foo')
      expect(ctx.getSync(['bar', 'bar'])).toEqual(undefined)
    })
+    it('should return pushed scope for in-place mutation', function () {
+      const scope = ctx.push({})
+      scope.item = 'ITEM'
+      expect(ctx.getSync(['item'])).toEqual('ITEM')
+    })
  })
  describe('.pop()', function () {
    it('should pop scope', async function () {
@@ -6,6 +6,8 @@ import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, isNu

 type PropertyKey = string | number;

+const BLOCKED_SCOPE_KEYS: ReadonlySet<PropertyKey> = new Set(['__proto__', 'constructor', 'prototype'])
+
 export class Context {
  /**
   * insert a Context-level empty scope,
@@ -94,8 +96,10 @@ export class Context {
    }
    return scope
  }
-  public push (ctx: object) {
-    return this.scopes.push(ctx)
+  public push (ctx: Scope): Scope {
+    const scope = createScope(ctx)
+    this.scopes.push(scope)
+    return scope
  }
  public pop () {
    return this.scopes.pop()
@@ -118,9 +122,9 @@ export class Context {
  private findScope (key: string | number) {
    for (let i = this.scopes.length - 1; i >= 0; i--) {
      const candidate = this.scopes[i]
-      if (key in candidate) return candidate
+      if (this.ownPropertyOnly ? hasOwnProperty.call(candidate, key) : key in candidate) return candidate
    }
-    if (key in this.environments) return this.environments
+    if (this.ownPropertyOnly ? hasOwnProperty.call(this.environments, key) : key in this.environments) return this.environments
    return this.globals
  }
  readProperty (obj: Scope, key: (PropertyKey | Drop)) {
@@ -131,30 +135,30 @@ 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)
-    else if (key === 'first') return readFirst(obj, this.ownPropertyOnly)
-    else if (key === 'last') return readLast(obj, this.ownPropertyOnly)
+    if (key === 'size') return this.readSize(obj)
+    else if (key === 'first') return this.readFirst(obj)
+    else if (key === 'last') return this.readLast(obj)
    return value
  }
+  private readFirst (obj: Scope) {
+    if (isArray(obj)) return readArrayElement(obj, 0, this.ownPropertyOnly)
+    return readJSProperty(obj, 'first', this.ownPropertyOnly)
+  }
+  private readLast (obj: Scope) {
+    if (isArray(obj)) return readArrayElement(obj, -1, this.ownPropertyOnly)
+    return readJSProperty(obj, 'last', this.ownPropertyOnly)
+  }
+  private readSize (obj: Scope) {
+    if (hasOwnProperty.call(obj, 'size')) return obj['size']
+    if (!this.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
+  }
 }

 export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean) {
+  if (BLOCKED_SCOPE_KEYS.has(key) && ownPropertyOnly) return undefined
  if (ownPropertyOnly && !hasOwnProperty.call(obj, key) && !(obj instanceof Drop)) return undefined
  return obj[key]
 }
-
-function readFirst (obj: Scope, ownPropertyOnly: boolean) {
-  if (isArray(obj)) return readArrayElement(obj, 0, ownPropertyOnly)
-  return readJSProperty(obj, 'first', ownPropertyOnly)
-}
-
-function readLast (obj: Scope, ownPropertyOnly: boolean) {
-  if (isArray(obj)) return readArrayElement(obj, -1, ownPropertyOnly)
-  return readJSProperty(obj, 'last', ownPropertyOnly)
-}
-
-function readSize (obj: Scope) {
-  if (hasOwnProperty.call(obj, 'size') || obj['size'] !== undefined) return obj['size']
-  if (isArray(obj) || isString(obj)) return obj.length
-  if (typeof obj === 'object') return Object.keys(obj).length
-}
@@ -6,8 +6,7 @@ export interface ScopeObject extends Record<string | number | symbol, any> {

 export type Scope = ScopeObject | Drop

-export function createScope (from?: ScopeObject): ScopeObject {
-  const scope = Object.create(null)
-  if (from) Object.assign(scope, from)
-  return scope
+export function createScope (from?: Scope): Scope {
+  if (from instanceof Drop) return from
+  return Object.assign(Object.create(null), from)
 }
@@ -38,10 +38,7 @@ export interface LiquidOptions {
  strictVariables?: boolean;
  /** Catch all errors instead of exit upon one. Please note that render errors won't be reached when parse fails. */
  catchAllErrors?: boolean;
-  /**
-   * Hide scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates.
-   * This only applies to property/index access on scope objects. Filter transforms and iteration operate on the resolved value with standard JavaScript semantics, so prototype-inherited array indices may still be surfaced by them.
-   */
+  /** Limit template property reads on plain scope objects to own properties. Defaults to `true`. See https://liquidjs.com/tutorials/security-model.html */
  ownPropertyOnly?: boolean;
  /** Modifies the behavior of `strictVariables`. If set, a single undefined variable will *not* cause an exception in the context of the `if`/`elsif`/`unless` tag and the `default` filter. Instead, it will evaluate to `false` and `null`, respectively. Irrelevant if `strictVariables` is not set. Defaults to `false`. **/
  lenientIf?: boolean;
@@ -1,4 +1,4 @@
-import { BlockMode, createScope } from '../context'
+import { BlockMode } from '../context'
 import { isTagToken } from '../util'
 import { BlockDrop } from '../drop'
 import { Liquid, TagToken, TopLevelToken, Template, Context, Emitter, Tag } from '..'
@@ -38,7 +38,7 @@ export default class extends Tag {
      if (stack.includes(self)) throw new Error('block tag cannot be nested')

      stack.push(self)
-      ctx.push(createScope({ block: superBlock }))
+      ctx.push({ block: superBlock })
      yield liquid.renderer.renderTemplates(templates, ctx, emitter)
      ctx.pop()
      stack.pop()
@@ -1,6 +1,5 @@
 import { Hash, ValueToken, Liquid, Tag, evalToken, Emitter, TagToken, TopLevelToken, Context, Template, ParseStream } from '..'
 import { assertEmpty, isValueToken, toEnumerable } from '../util'
-import { createScope } from '../context/scope'
 import { ForloopDrop } from '../drop/forloop-drop'
 import { Parser } from '../parser'
 import { Arguments } from '../template'
@@ -44,7 +43,7 @@ export default class extends Tag {
  * render (ctx: Context, emitter: Emitter): Generator<unknown, void | string, Template[]> {
    const r = this.liquid.renderer
    const continueKey = 'continue-' + this.variable + '-' + this.collection.getText()
-    ctx.push(createScope({ continue: ctx.getRegister(continueKey, {}) }))
+    ctx.push({ continue: ctx.getRegister(continueKey, {}) })
    const hash = (yield this.hash.render(ctx)) as Record<string, any>
    ctx.pop()

@@ -68,8 +67,7 @@ export default class extends Tag {

    if (!this.templates.length) return

-    const scope = createScope({ forloop: new ForloopDrop(collection.length, this.collection.getText(), this.variable) })
-    ctx.push(scope)
+    const scope = ctx.push({ forloop: new ForloopDrop(collection.length, this.collection.getText(), this.variable) })
    for (const item of collection) {
      scope[this.variable] = item
      ctx.continueCalled = ctx.breakCalled = false
@@ -1,5 +1,5 @@
 import { Template, ValueToken, TopLevelToken, Liquid, Tag, assert, evalToken, Hash, Emitter, TagToken, Context } from '..'
-import { BlockMode, createScope, Scope } from '../context'
+import { BlockMode, Scope } from '../context'
 import { Parser } from '../parser'
 import { Argument, Arguments, PartialScope } from '../template'
 import { isString, isValueToken } from '../util'
@@ -37,10 +37,10 @@ export default class extends Tag {
    const saved = ctx.saveRegister('blocks', 'blockMode')
    ctx.setRegister('blocks', {})
    ctx.setRegister('blockMode', BlockMode.OUTPUT)
-    const scope = createScope((yield hash.render(ctx)) as Scope)
+    const scope = (yield hash.render(ctx)) as Scope
    if (withVar) scope[filepath] = yield evalToken(withVar, ctx)
    const templates = (yield liquid._parsePartialFile(filepath, ctx.sync, this.currentFile)) as Template[]
-    ctx.push(ctx.opts.jekyllInclude ? createScope({ include: scope }) : scope)
+    ctx.push(ctx.opts.jekyllInclude ? { include: scope } : scope)
    yield renderer.renderTemplates(templates, ctx, emitter)
    ctx.pop()
    ctx.restoreRegister(saved)
@@ -1,5 +1,5 @@
 import { Scope, Template, Liquid, Tag, assert, Emitter, Hash, TagToken, TopLevelToken, Context } from '..'
-import { BlockMode, createScope } from '../context'
+import { BlockMode } from '../context'
 import { parseFilePath, renderFilePath, ParsedFileName } from './render'
 import { BlankDrop } from '../drop'
 import { Parser } from '../parser'
@@ -41,7 +41,7 @@ export default class extends Tag {
    ctx.setRegister('blockMode', BlockMode.OUTPUT)

    // render the layout file use stored blocks
-    ctx.push(createScope((yield args.render(ctx)) as Scope))
+    ctx.push((yield args.render(ctx)) as Scope)
    yield renderer.renderTemplates(templates, ctx, emitter)
    ctx.pop()
    ctx.depthLimit.release(1)
@@ -1,5 +1,4 @@
 import { isValueToken, toEnumerable } from '../util'
-import { createScope } from '../context/scope'
 import { ValueToken, Liquid, Tag, evalToken, Emitter, Hash, TagToken, TopLevelToken, Context, Template, ParseStream } from '..'
 import { TablerowloopDrop } from '../drop/tablerowloop-drop'
 import { Parser } from '../parser'
@@ -53,8 +52,7 @@ export default class extends Tag {

    const r = this.liquid.renderer
    const tablerowloop = new TablerowloopDrop(collection.length, cols, this.collection.getText(), this.variable)
-    const scope = createScope({ tablerowloop })
-    ctx.push(scope)
+    const scope = ctx.push({ tablerowloop })

    for (let idx = 0; idx < collection.length; idx++, tablerowloop.next()) {
      scope[this.variable] = collection[idx]
@@ -0,0 +1,61 @@
+import { Liquid } from '../../../src/liquid'
+import { Drop } from '../../../src/drop/drop'
+
+describe('scope security', function () {
+  let liquid: Liquid
+
+  beforeEach(function () {
+    liquid = new Liquid()
+  })
+
+  it('should iterate plain objects via inherited Symbol.iterator (ownPropertyOnly exception)', 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('inherited')
+    } 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 read inherited size when ownPropertyOnly=false', async function () {
+    liquid = new Liquid({ ownPropertyOnly: false })
+    const obj = Object.create({ size: 99 })
+    await expect(liquid.parseAndRender('{{ obj.size }}', { obj })).resolves.toBe('99')
+  })
+
+  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')
+  })
+
+  it('should block own blocked keys when ownPropertyOnly=true', async function () {
+    const scope = JSON.parse('{"__proto__": {"polluted": true}, "constructor": {"name": "Custom"}, "name": "Alice"}')
+    await expect(liquid.parseAndRender('{{ __proto__.polluted }}', scope)).resolves.toBe('')
+    await expect(liquid.parseAndRender('{{ constructor.name }}', scope)).resolves.toBe('')
+    await expect(liquid.parseAndRender('{{ name }}', scope)).resolves.toBe('Alice')
+  })
+
+  it('should block inherited properties when ownPropertyOnly=true', async function () {
+    const scope = { foo: Object.create({ __proto__: { bar: 'BAR' }, constructor: { name: 'Evil' } }) }
+    await expect(liquid.parseAndRender('{{ foo.__proto__ }}', scope)).resolves.toBe('')
+    await expect(liquid.parseAndRender('{{ foo.constructor }}', scope)).resolves.toBe('')
+  })
+})