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

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

docs/source/tutorials/security-model.md

@@ -52,6 +52,8 @@ The `memoryLimit` option was removed in v11; enforce memory limits at the host o
 
 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.
 
+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)).
+
 ## Custom `Drop` classes
 
 [`Drop`][drop] values are not restricted the same way: LiquidJS still reads the prototype chain and may call [`liquidMethodMissing`][liquidMethodMissing]. **You** control what a drop exposes; narrow APIs and never feed unsafe data into drops unless the class is built for template access. `ownPropertyOnly` alone does not harden custom drops—audit them like any privileged code.

src/context/context.spec.ts

@@ -198,6 +198,23 @@ describe('Context', function () {
         delete (Array.prototype as any)[0]
       }
     })
+    it('should block __proto__ access', function () {
+      ctx.push({ foo: { __proto__: { bar: 'BAR' } } })
+      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 () {

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, isBlockedScopeKey, Scope } from './scope'
 import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, isNumber, toLiquid, InternalUndefinedVariableError, toValueSync, isObject, Limiter, toValue, readArrayElement } from '../util'
 
 type PropertyKey = string | number;
@@ -116,11 +116,19 @@ export class Context {
     })
   }
   private findScope (key: string | number) {
+    if (isBlockedScopeKey(key)) return createScope()
+    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
+    if (hasKey(this.globals)) return this.globals
     return this.globals
   }
   readProperty (obj: Scope, key: (PropertyKey | Drop)) {
@@ -139,6 +147,7 @@ export class Context {
 }
 
 export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean) {
+  if (isBlockedScopeKey(key)) return undefined
   if (ownPropertyOnly && !hasOwnProperty.call(obj, key) && !(obj instanceof Drop)) return undefined
   return obj[key]
 }

src/context/scope.ts

@@ -1,4 +1,5 @@
 import { Drop } from '../drop/drop'
+import { hasOwnProperty } from '../util'
 
 export interface ScopeObject extends Record<string | number | symbol, any> {
   toLiquid?: () => any;
@@ -6,8 +7,22 @@ export interface ScopeObject extends Record<string | number | symbol, 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 createScope (from?: ScopeObject): ScopeObject {
+  return from ? sanitizeScope(from) : Object.create(null)
+}
+
+export function sanitizeScope (obj: ScopeObject): ScopeObject {
   const scope = Object.create(null)
-  if (from) Object.assign(scope, from)
+  for (const key of Object.keys(obj)) {
+    if (!isBlockedScopeKey(key) && hasOwnProperty.call(obj, key)) {
+      scope[key] = obj[key]
+    }
+  }
   return scope
 }

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

@@ -0,0 +1,58 @@
+import { Liquid } from '../../../src/liquid'
+
+describe('scope security', function () {
+  let liquid: Liquid
+
+  beforeEach(function () {
+    liquid = new Liquid()
+  })
+
+  it('should not read __proto__ from passed scope', async function () {
+    const scope = JSON.parse('{"__proto__": {"polluted": true}, "name": "Alice"}')
+    await expect(liquid.parseAndRender('{{ name }}', scope)).resolves.toBe('Alice')
+    await expect(liquid.parseAndRender('{{ __proto__.polluted }}', scope)).resolves.toBe('')
+  })
+
+  it('should not read constructor from passed scope', async function () {
+    const scope = { name: 'Alice', constructor: { name: 'Object' } }
+    await expect(liquid.parseAndRender('{{ constructor.name }}', scope)).resolves.toBe('')
+  })
+
+  it('should not read prototype chain properties by default', async function () {
+    const scope = { user: Object.create({ isAdmin: true, name: 'Inherited' }) }
+    scope.user.name = 'Alice'
+    await expect(liquid.parseAndRender('{{ user.name }}', scope)).resolves.toBe('Alice')
+    await expect(liquid.parseAndRender('{{ user.isAdmin }}', scope)).resolves.toBe('')
+  })
+
+  it('should not expose Object.prototype keys from polluted scope', async function () {
+    const scope = Object.create({ polluted: 'yes' })
+    scope.safe = 'ok'
+    await expect(liquid.parseAndRender('{{ safe }}', scope)).resolves.toBe('ok')
+    await expect(liquid.parseAndRender('{{ polluted }}', scope)).resolves.toBe('')
+  })
+
+  it('should block assign to __proto__ from being read back', async function () {
+    await expect(liquid.parseAndRender(
+      '{% assign __proto__ = obj %}{{ __proto__.polluted }}',
+      { obj: { polluted: true } }
+    )).resolves.toBe('')
+  })
+
+  it('should still allow increment on user scope', async function () {
+    const scope = { counter: 0 }
+    await expect(liquid.parseAndRender('{% increment counter %}', scope)).resolves.toBe('0')
+    await expect(liquid.parseAndRender('{% increment counter %}', scope)).resolves.toBe('1')
+    expect(scope.counter).toBe(2)
+  })
+
+  it('should allow ownPropertyOnly=false to read prototype values', async function () {
+    const scope = { foo: Object.create({ bar: 'BAR' }) }
+    await expect(liquid.parseAndRender('{{ foo.bar }}', scope, { ownPropertyOnly: false })).resolves.toBe('BAR')
+  })
+
+  it('should still block __proto__ when ownPropertyOnly=false', async function () {
+    const scope = { foo: { __proto__: { bar: 'BAR' } } }
+    await expect(liquid.parseAndRender('{{ foo.__proto__.bar }}', scope, { ownPropertyOnly: false })).resolves.toBe('')
+  })
+})