diff --git a/docs/source/tutorials/security-model.md b/docs/source/tutorials/security-model.md index 9a6131e8b..c588cf2c3 100644 --- a/docs/source/tutorials/security-model.md +++ b/docs/source/tutorials/security-model.md @@ -52,7 +52,7 @@ The `memoryLimit` option was removed in v11; enforce memory limits at the host o [`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). +**Proto-related keys** (`__proto__`, `constructor`, `prototype`): when [`ownPropertyOnly`][ownPropertyOnly] is `true` (default), template reads of 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: diff --git a/src/context/scope.ts b/src/context/scope.ts index 8b2a4626f..9ae157d20 100644 --- a/src/context/scope.ts +++ b/src/context/scope.ts @@ -9,7 +9,7 @@ export type Scope = ScopeObject | Drop const BLOCKED_SCOPE_KEYS = new Set(['__proto__', 'constructor', 'prototype']) -export function isBlockedScopeKey (key: PropertyKey): boolean { +function isBlockedScopeKey (key: PropertyKey): boolean { return typeof key === 'string' && BLOCKED_SCOPE_KEYS.has(key) } @@ -19,10 +19,6 @@ export function shouldBlockScopeKeyRead (obj: Scope, key: PropertyKey, ownProper return !hasOwnProperty.call(obj, key) } -export function shouldBlockScopeKeyWrite (key: PropertyKey, ownPropertyOnly: boolean): boolean { - return ownPropertyOnly && isBlockedScopeKey(key) -} - export function createScope (from?: ScopeObject): ScopeObject { return Object.assign(Object.create(null), from) } diff --git a/src/tags/assign.ts b/src/tags/assign.ts index 8de8756d3..da6114120 100644 --- a/src/tags/assign.ts +++ b/src/tags/assign.ts @@ -1,5 +1,4 @@ import { Value, Liquid, TopLevelToken, TagToken, Context, Tag } from '..' -import { shouldBlockScopeKeyWrite } from '../context/scope' import { Arguments } from '../template' import { IdentifierToken } from '../tokens' @@ -21,7 +20,6 @@ export default class extends Tag { this.value = new Value(this.tokenizer.readFilteredValue(), this.liquid) } * render (ctx: Context): Generator { - if (shouldBlockScopeKeyWrite(this.key, ctx.ownPropertyOnly)) return ctx.bottom()[this.key] = yield this.value.value(ctx, this.liquid.options.lenientIf) } diff --git a/src/tags/capture.ts b/src/tags/capture.ts index 5ddacc54b..d82f14a62 100644 --- a/src/tags/capture.ts +++ b/src/tags/capture.ts @@ -1,5 +1,4 @@ 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' @@ -32,7 +31,6 @@ export default class extends Tag { * render (ctx: Context): Generator { 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 } diff --git a/src/tags/decrement.ts b/src/tags/decrement.ts index f0caf8113..c854e2481 100644 --- a/src/tags/decrement.ts +++ b/src/tags/decrement.ts @@ -1,5 +1,4 @@ import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..' -import { shouldBlockScopeKeyWrite } from '../context/scope' import { IdentifierToken } from '../tokens' import { isNumber, stringify } from '../util' @@ -12,7 +11,6 @@ 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 diff --git a/src/tags/increment.ts b/src/tags/increment.ts index ad537e336..948faf0ce 100644 --- a/src/tags/increment.ts +++ b/src/tags/increment.ts @@ -1,5 +1,4 @@ import { isNumber, stringify } from '../util' -import { shouldBlockScopeKeyWrite } from '../context/scope' import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..' import { IdentifierToken } from '../tokens' @@ -12,7 +11,6 @@ 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 diff --git a/test/integration/liquid/scope-security.spec.ts b/test/integration/liquid/scope-security.spec.ts index 8bf7cf0ee..6bb6a483f 100644 --- a/test/integration/liquid/scope-security.spec.ts +++ b/test/integration/liquid/scope-security.spec.ts @@ -19,25 +19,11 @@ describe('scope security', function () { await expect(liquid.parseAndRender('{{ constructor.name }}', scope)).resolves.toBe('') }) - it('should block assign to __proto__', async function () { - await expect(liquid.parseAndRender( - '{% assign __proto__ = obj %}{{ __proto__.polluted }}', - { obj: { polluted: true } } - )).resolves.toBe('') - expect((Object.prototype as any).polluted).toBeUndefined() - }) - it('should block inherited constructor when ownPropertyOnly=false', async function () { await expect(liquid.parseAndRender('{{ foo.constructor.name }}', { foo: {} }, { ownPropertyOnly: false })).resolves.toBe('') await expect(liquid.parseAndRender('{{ constructor.name }}', { name: 'Alice' }, { ownPropertyOnly: false })).resolves.toBe('') }) - it('should not write increment to __proto__ on user scope', async function () { - const scope = Object.create(null) as Record - await expect(liquid.parseAndRender('{% increment __proto__ %}', scope)).resolves.toBe('') - expect(scope).toEqual({}) - }) - it('should iterate plain objects via inherited Symbol.iterator (ownPropertyOnly exception)', async function () { // eslint-disable-next-line no-extend-native (Object.prototype as any)[Symbol.iterator] = function * () { yield 'inherited' }