feat: block dangerous scope keys and harden findScope (#898)

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`. Proto-related keys (`__proto__`, `constructor`, `prototype`) are blocked when `ownPropertyOnly` is `true` (even as own properties); when `false`, own properties with those names are allowed and inherited prototype-chain access to those names is still blocked. [`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,7 +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 `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.
+[`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.
+
+**Proto-related keys** (`__proto__`, `constructor`, `prototype`): when [`ownPropertyOnly`][ownPropertyOnly] is `true` (default), template reads and writes to those names are blocked even if they are own properties—this defends against prototype pollution from sources such as `JSON.parse('{"__proto__":…}')`. When `ownPropertyOnly` is `false`, own properties with those names are allowed; inherited prototype-chain access to those names is still blocked. Managed scopes use null prototypes (loop locals, `{% render %}` bindings, filter iteration scopes).
+
+**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 %}`): uses `Symbol.iterator` when present, including inherited iterators on plain objects; class instances and drops keep their iterators too.
+- 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.spec.ts

@@ -1,4 +1,5 @@
 import { Context } from './context'
+import { Drop } from '../drop/drop'
 import { Scope } from './scope'
 
 describe('Context', function () {
@@ -198,6 +199,35 @@ 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 still block inherited blocked keys when ownPropertyOnly=false', function () {
+      ctx = new Context({ foo: Object.create({ __proto__: { bar: 'BAR' } }) }, { ownPropertyOnly: false } as any)
+      expect(ctx.getSync(['foo', '__proto__'])).toEqual(undefined)
+    })
+    it('should block constructor access', function () {
+      ctx.push({ foo: { constructor: { name: 'Evil' } } })
+      expect(ctx.getSync(['foo', 'constructor'])).toEqual(undefined)
+    })
+    it('should block prototype access', function () {
+      ctx.push({ foo: { prototype: { bar: 'BAR' } } })
+      expect(ctx.getSync(['foo', 'prototype'])).toEqual(undefined)
+    })
+    it('should block top-level __proto__ variable', function () {
+      ctx = new Context({ __proto__: { bar: 'BAR' }, bar: 'BAR' } as any)
+      expect(ctx.getSync(['__proto__'])).toEqual(undefined)
+      expect(ctx.getSync(['bar'])).toEqual('BAR')
+    })
   })
 
   describe('.getAll()', function () {
@@ -221,6 +251,21 @@ describe('Context', function () {
       expect(ctx.getSync(['bar', 'foo'])).toEqual('foo')
       expect(ctx.getSync(['bar', 'bar'])).toEqual(undefined)
     })
+    it('should wrap plain objects with null prototype', function () {
+      const scope = ctx.push({ foo: 'FOO' })
+      expect(Object.getPrototypeOf(scope)).toBeNull()
+    })
+    it('should return pushed scope for in-place mutation', function () {
+      const scope = ctx.push({})
+      scope.item = 'ITEM'
+      expect(ctx.getSync(['item'])).toEqual('ITEM')
+    })
+    it('should push Drop instances as-is', function () {
+      class TestDrop extends Drop {}
+      const drop = new TestDrop()
+      const pushed = ctx.push(drop)
+      expect(pushed).toBe(drop)
+    })
   })
   describe('.pop()', function () {
     it('should pop scope', async function () {

src/context/context.ts

@@ -1,7 +1,7 @@
 import { Drop } from '../drop/drop'
 import { __assign } from 'tslib'
 import { NormalizedFullOptions, defaultOptions, RenderOptions } from '../liquid-options'
-import { createScope, Scope } from './scope'
+import { createScope, Scope, shouldBlockScopeKeyRead } from './scope'
 import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, isNumber, toLiquid, InternalUndefinedVariableError, toValueSync, isObject, Limiter, toValue, readArrayElement } from '../util'
 
 type PropertyKey = string | number;
@@ -94,8 +94,14 @@ export class Context {
     }
     return scope
   }
-  public push (ctx: object) {
-    return this.scopes.push(ctx)
+  public push (ctx: Scope): Scope {
+    const scope = ctx instanceof Drop
+      ? ctx
+      : Object.getPrototypeOf(ctx) === null
+        ? ctx
+        : createScope(ctx)
+    this.scopes.push(scope)
+    return scope
   }
   public pop () {
     return this.scopes.pop()
@@ -116,11 +122,17 @@ export class Context {
     })
   }
   private findScope (key: string | number) {
+    const hasKey = (obj: Scope) => {
+      if (obj == null) return false
+      return this.ownPropertyOnly
+        ? hasOwnProperty.call(obj, key)
+        : key in obj
+    }
     for (let i = this.scopes.length - 1; i >= 0; i--) {
       const candidate = this.scopes[i]
-      if (key in candidate) return candidate
+      if (hasKey(candidate)) return candidate
     }
-    if (key in this.environments) return this.environments
+    if (hasKey(this.environments)) return this.environments
     return this.globals
   }
   readProperty (obj: Scope, key: (PropertyKey | Drop)) {
@@ -131,30 +143,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 (shouldBlockScopeKeyRead(obj, 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

@@ -1,13 +1,28 @@
 import { Drop } from '../drop/drop'
+import { hasOwnProperty } from '../util'
 
 export interface ScopeObject extends Record<string | number | symbol, any> {
   toLiquid?: () => any;
 }
 
 export type Scope = ScopeObject | Drop
 
+const BLOCKED_SCOPE_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
+
+export function isBlockedScopeKey (key: PropertyKey): boolean {
+  return typeof key === 'string' && BLOCKED_SCOPE_KEYS.has(key)
+}
+
+export function shouldBlockScopeKeyRead (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean): boolean {
+  if (!isBlockedScopeKey(key)) return false
+  if (ownPropertyOnly) return true
+  return !hasOwnProperty.call(obj, key)
+}
+
+export function shouldBlockScopeKeyWrite (key: PropertyKey, ownPropertyOnly: boolean): boolean {
+  return ownPropertyOnly && isBlockedScopeKey(key)
+}
+
 export function createScope (from?: ScopeObject): ScopeObject {
-  const scope = Object.create(null)
-  if (from) Object.assign(scope, from)
-  return scope
+  return Object.assign(Object.create(null), from)
 }

src/tags/assign.ts

@@ -1,4 +1,5 @@
 import { Value, Liquid, TopLevelToken, TagToken, Context, Tag } from '..'
+import { shouldBlockScopeKeyWrite } 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 (shouldBlockScopeKeyWrite(this.key, ctx.ownPropertyOnly)) return
     ctx.bottom()[this.key] = yield this.value.value(ctx, this.liquid.options.lenientIf)
   }
 

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/capture.ts

@@ -1,4 +1,5 @@
 import { Liquid, Tag, Template, Context, TagToken, TopLevelToken } from '..'
+import { shouldBlockScopeKeyWrite } 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 (shouldBlockScopeKeyWrite(this.variable, ctx.ownPropertyOnly)) return
     ctx.bottom()[this.variable] = html
   }
 

src/tags/decrement.ts

@@ -1,4 +1,5 @@
 import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..'
+import { shouldBlockScopeKeyWrite } 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 (shouldBlockScopeKeyWrite(this.variable, context.ownPropertyOnly)) return
     const scope = context.environments
     if (!isNumber(scope[this.variable])) {
       scope[this.variable] = 0

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/increment.ts

@@ -1,4 +1,5 @@
 import { isNumber, stringify } from '../util'
+import { shouldBlockScopeKeyWrite } 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 (shouldBlockScopeKeyWrite(this.variable, context.ownPropertyOnly)) return
     const scope = context.environments
     if (!isNumber(scope[this.variable])) {
       scope[this.variable] = 0

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]