From 047b110939f13d6eb04ea17e382b4e0f6518da85 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Thu, 23 Jul 2026 23:14:15 +0800 Subject: [PATCH] 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 --- docs/source/tutorials/options.md | 2 +- docs/source/tutorials/security-model.md | 2 +- src/context/context.spec.ts | 12 ++++++------ src/context/context.ts | 3 +++ src/liquid-options.ts | 2 +- test/integration/liquid/scope-security.spec.ts | 7 ++++--- 6 files changed, 16 insertions(+), 12 deletions(-) diff --git a/docs/source/tutorials/options.md b/docs/source/tutorials/options.md index 7dbede068..3eacdc191 100644 --- a/docs/source/tutorials/options.md +++ b/docs/source/tutorials/options.md @@ -138,7 +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** limits template property reads on plain scope objects to own properties (no inherited prototype keys). Defaults to `true`. With `ownPropertyOnly: false`, inherited properties are allowed. Sanitize untrusted input (e.g. with [bourne](https://www.npmjs.com/package/bourne)) before passing it as scope. See [Security Model](./security-model.html). +**ownPropertyOnly** limits template property reads on plain scope objects to own properties (no inherited prototype keys). Defaults to `true`. When `true`, reads of `__proto__`, `constructor`, and `prototype` are blocked (own and inherited) as a prototype-pollution defense. With `ownPropertyOnly: false`, inherited properties and those keys are allowed—sanitize untrusted input (e.g. with [bourne](https://www.npmjs.com/package/bourne)) before passing it as scope. See [Security Model](./security-model.html). {% note info Nonexistent Tags %} Nonexistent tags always throw errors during parsing and this behavior cannot be customized. diff --git a/docs/source/tutorials/security-model.md b/docs/source/tutorials/security-model.md index 16b6fa900..454bdf57a 100644 --- a/docs/source/tutorials/security-model.md +++ b/docs/source/tutorials/security-model.md @@ -50,7 +50,7 @@ The `memoryLimit` option was removed in v11; enforce memory limits at the host o ## `ownPropertyOnly` and scope data -With [`ownPropertyOnly`][ownPropertyOnly] `true` (default), plain scope objects only expose **own** properties (no inherited / `Object.prototype` keys). With `false`, inherited properties 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`. +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. diff --git a/src/context/context.spec.ts b/src/context/context.spec.ts index cd6eaee4c..839810c8e 100644 --- a/src/context/context.spec.ts +++ b/src/context/context.spec.ts @@ -215,17 +215,17 @@ describe('Context', function () { expect(ctx.getSync(['foo', '__proto__', '__proto__', 'bar'])).toEqual('BAR') expect(ctx.getSync(['foo', 'constructor', 'name'])).toEqual('Evil') }) - it('should allow own constructor when ownPropertyOnly=true', function () { + it('should block own constructor when ownPropertyOnly=true', function () { ctx.push({ foo: { constructor: { name: 'Evil' } } }) - expect(ctx.getSync(['foo', 'constructor'])).toEqual({ name: 'Evil' }) + expect(ctx.getSync(['foo', 'constructor'])).toEqual(undefined) }) - it('should allow own prototype when ownPropertyOnly=true', function () { + it('should block own prototype when ownPropertyOnly=true', function () { ctx.push({ foo: { prototype: { bar: 'BAR' } } }) - expect(ctx.getSync(['foo', 'prototype'])).toEqual({ bar: 'BAR' }) + expect(ctx.getSync(['foo', 'prototype'])).toEqual(undefined) }) - it('should allow own top-level __proto__ variable', function () { + 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({ bar: 'BAR' }) + expect(ctx.getSync(['__proto__'])).toEqual(undefined) expect(ctx.getSync(['bar'])).toEqual('BAR') }) }) diff --git a/src/context/context.ts b/src/context/context.ts index 41976491e..94a8301c8 100644 --- a/src/context/context.ts +++ b/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 = new Set(['__proto__', 'constructor', 'prototype']) + export class Context { /** * insert a Context-level empty scope, @@ -156,6 +158,7 @@ export class Context { } 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] } diff --git a/src/liquid-options.ts b/src/liquid-options.ts index 49fc10d14..843e9de1e 100644 --- a/src/liquid-options.ts +++ b/src/liquid-options.ts @@ -38,7 +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; - /** Limit template property reads on plain scope objects to own properties (no inherited prototype keys). Defaults to `true`. Sanitize untrusted scope data (e.g. with [bourne](https://www.npmjs.com/package/bourne)) before passing it as scope. */ + /** Limit template property reads on plain scope objects to own properties (no inherited prototype keys). Defaults to `true`. When `true`, reads of `__proto__`, `constructor`, and `prototype` are blocked. 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. */ 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; diff --git a/test/integration/liquid/scope-security.spec.ts b/test/integration/liquid/scope-security.spec.ts index 34dd500cc..2475421d7 100644 --- a/test/integration/liquid/scope-security.spec.ts +++ b/test/integration/liquid/scope-security.spec.ts @@ -40,10 +40,11 @@ describe('scope security', function () { )).resolves.toBe('ab') }) - it('should allow own blocked keys when ownPropertyOnly=true', async function () { + 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('true') - await expect(liquid.parseAndRender('{{ constructor.name }}', scope)).resolves.toBe('Custom') + 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 () {