From 85321c6d64db44d34047e57e0d064b5f5f07bbf6 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Thu, 23 Jul 2026 22:59:37 +0800 Subject: [PATCH] fix: apply ownPropertyOnly uniformly in readJSProperty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proto keys block inherited access only; ownPropertyOnly is checked once before return for all keys. Own __proto__/constructor/prototype properties are readable—sanitize untrusted scope input. Co-authored-by: Cursor --- docs/source/tutorials/options.md | 2 +- docs/source/tutorials/security-model.md | 5 +++-- src/context/context.spec.ts | 14 +++++++------- src/context/context.ts | 5 +---- test/integration/liquid/scope-security.spec.ts | 12 ++++++++++++ 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/docs/source/tutorials/options.md b/docs/source/tutorials/options.md index 56adde3ee..8ea4ebaa7 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`, which blocks proto keys (`__proto__`, `constructor`, `prototype`) entirely; with `false`, only inherited proto-key access is blocked. 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`. Inherited access to proto keys (`__proto__`, `constructor`, `prototype`) is always blocked; with `ownPropertyOnly: false`, other inherited properties are allowed. Own properties named `__proto__`, `constructor`, or `prototype` are readable—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 974384846..df3914b83 100644 --- a/docs/source/tutorials/security-model.md +++ b/docs/source/tutorials/security-model.md @@ -52,8 +52,9 @@ The `memoryLimit` option was removed in v11; enforce memory limits at the host o With [`ownPropertyOnly`][ownPropertyOnly] `true` (default), plain scope objects only expose **own** properties (no inherited / `Object.prototype` keys). -- **`true`:** proto keys (`__proto__`, `constructor`, `prototype`) blocked entirely. -- **`false`:** own properties with those names allowed; inherited proto-key access still blocked. +- **Proto keys** (`__proto__`, `constructor`, `prototype`): inherited access is always blocked. +- **`true`:** other inherited properties are also hidden. +- **`false`:** other inherited properties are allowed; own properties named `__proto__`, `constructor`, or `prototype` remain readable. Sanitize untrusted scope data (e.g. with [bourne](https://www.npmjs.com/package/bourne)) if those key names may appear. 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 6cac6d84b..80d4ffe31 100644 --- a/src/context/context.spec.ts +++ b/src/context/context.spec.ts @@ -214,17 +214,17 @@ describe('Context', 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 () { + it('should allow own constructor when ownPropertyOnly=true', function () { ctx.push({ foo: { constructor: { name: 'Evil' } } }) - expect(ctx.getSync(['foo', 'constructor'])).toEqual(undefined) + expect(ctx.getSync(['foo', 'constructor'])).toEqual({ name: 'Evil' }) }) - it('should block prototype access', function () { + it('should allow own prototype when ownPropertyOnly=true', function () { ctx.push({ foo: { prototype: { bar: 'BAR' } } }) - expect(ctx.getSync(['foo', 'prototype'])).toEqual(undefined) + expect(ctx.getSync(['foo', 'prototype'])).toEqual({ bar: 'BAR' }) }) - it('should block top-level __proto__ variable', function () { - ctx = new Context({ __proto__: { bar: 'BAR' }, bar: 'BAR' } as any) - expect(ctx.getSync(['__proto__'])).toEqual(undefined) + it('should allow own top-level __proto__ variable', function () { + ctx = new Context(JSON.parse('{"__proto__": {"bar": "BAR"}, "bar": "BAR"}')) + expect(ctx.getSync(['__proto__'])).toEqual({ bar: 'BAR' }) expect(ctx.getSync(['bar'])).toEqual('BAR') }) }) diff --git a/src/context/context.ts b/src/context/context.ts index 872088615..428b3a45d 100644 --- a/src/context/context.ts +++ b/src/context/context.ts @@ -158,10 +158,7 @@ export class Context { const BLOCKED_SCOPE_KEYS: ReadonlySet = new Set(['__proto__', 'constructor', 'prototype']) export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean) { - if (BLOCKED_SCOPE_KEYS.has(key)) { - if (!hasOwnProperty.call(obj, key)) return undefined - if (ownPropertyOnly) return undefined - } + if (BLOCKED_SCOPE_KEYS.has(key) && !hasOwnProperty.call(obj, key)) return undefined if (ownPropertyOnly && !hasOwnProperty.call(obj, key) && !(obj instanceof Drop)) return undefined return obj[key] } diff --git a/test/integration/liquid/scope-security.spec.ts b/test/integration/liquid/scope-security.spec.ts index 2c3ae8973..40194cc08 100644 --- a/test/integration/liquid/scope-security.spec.ts +++ b/test/integration/liquid/scope-security.spec.ts @@ -39,4 +39,16 @@ describe('scope security', function () { { drop: new IterableDrop() } )).resolves.toBe('ab') }) + + it('should allow 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') + }) + + it('should still block inherited blocked keys when ownPropertyOnly=true', async function () { + const scope = { foo: Object.create({ __proto__: { bar: 'BAR' }, constructor: { name: 'Evil' } }) } + await expect(liquid.parseAndRender('{{ foo.__proto__.bar }}', scope)).resolves.toBe('') + await expect(liquid.parseAndRender('{{ foo.constructor.name }}', scope)).resolves.toBe('') + }) })