From 6bdf65a6a13b49a517cabe1ce1f95baeb39cc7ed Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Wed, 15 Jul 2026 23:02:06 +0800 Subject: [PATCH] feat: block dangerous scope keys and harden findScope (#898) Co-authored-by: Cursor --- docs/source/tutorials/security-model.md | 2 + src/context/context.spec.ts | 17 ++++++ src/context/context.ts | 15 ++++- src/context/scope.ts | 17 +++++- .../integration/liquid/scope-security.spec.ts | 58 +++++++++++++++++++ 5 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 test/integration/liquid/scope-security.spec.ts diff --git a/docs/source/tutorials/security-model.md b/docs/source/tutorials/security-model.md index 3c6b4aa74..db6002ff2 100644 --- a/docs/source/tutorials/security-model.md +++ b/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. diff --git a/src/context/context.spec.ts b/src/context/context.spec.ts index d8eeac978..035687f62 100644 --- a/src/context/context.spec.ts +++ b/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 () { diff --git a/src/context/context.ts b/src/context/context.ts index fbc17f8d8..ae76d838d 100644 --- a/src/context/context.ts +++ b/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] } diff --git a/src/context/scope.ts b/src/context/scope.ts index 2cd261588..825d4b103 100644 --- a/src/context/scope.ts +++ b/src/context/scope.ts @@ -1,4 +1,5 @@ import { Drop } from '../drop/drop' +import { hasOwnProperty } from '../util' export interface ScopeObject extends Record { toLiquid?: () => any; @@ -6,8 +7,22 @@ export interface ScopeObject extends Record { 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 } diff --git a/test/integration/liquid/scope-security.spec.ts b/test/integration/liquid/scope-security.spec.ts new file mode 100644 index 000000000..c8d10472f --- /dev/null +++ b/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('') + }) +})