test: cover readSize branches in Context

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. 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.

docs/source/tutorials/security-model.md

@@ -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
 

src/context/context.spec.ts

@@ -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 () {

src/context/context.ts

@@ -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
-}

src/context/scope.ts

@@ -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)
 }

src/liquid-options.ts

@@ -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;

src/tags/block.ts

@@ -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()

src/tags/for.ts

@@ -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

src/tags/include.ts

@@ -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)

src/tags/layout.ts

@@ -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)

src/tags/tablerow.ts

@@ -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]

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

@@ -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('')
+  })
+})