diff --git a/docs/source/tutorials/security-model.md b/docs/source/tutorials/security-model.md index 6e90430dd..af53c82ed 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. -**Always blocked** (regardless of `ownPropertyOnly`): template access to the property names `__proto__`, `constructor`, and `prototype`, and writes to those names via `{% assign %}`, `{% capture %}`, `{% increment %}`, and `{% decrement %}`. Managed scopes built with null prototypes (loop locals, `{% render %}` bindings, filter iteration scopes) omit those keys when created from user data. +**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). **Exceptions** — `ownPropertyOnly` does not restrict: diff --git a/src/context/context.spec.ts b/src/context/context.spec.ts index 035687f62..e9262bc06 100644 --- a/src/context/context.spec.ts +++ b/src/context/context.spec.ts @@ -198,8 +198,20 @@ describe('Context', function () { delete (Array.prototype as any)[0] } }) - it('should block __proto__ access', function () { - ctx.push({ foo: { __proto__: { bar: 'BAR' } } }) + it('should allow own blocked keys when ownPropertyOnly=false', function () { + ctx = new Context({ + foo: { + ...JSON.parse('{"__proto__": {"bar": "BAR"}}'), + constructor: { name: 'Custom' }, + prototype: { x: 1 } + } + }, { ownPropertyOnly: false } as any) + expect(ctx.getSync(['foo', '__proto__', 'bar'])).toEqual('BAR') + expect(ctx.getSync(['foo', 'constructor', 'name'])).toEqual('Custom') + expect(ctx.getSync(['foo', 'prototype', 'x'])).toEqual(1) + }) + it('should still block inherited blocked keys when ownPropertyOnly=false', 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 () { diff --git a/src/context/context.ts b/src/context/context.ts index 9ab03798c..471e29760 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, isBlockedScopeKey, Scope } from './scope' +import { createScope, isBlockedScopeKey, Scope, shouldBlockScopeKeyRead } from './scope' import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, isNumber, toLiquid, InternalUndefinedVariableError, toValueSync, isObject, Limiter, toValue, readArrayElement } from '../util' type PropertyKey = string | number; @@ -116,7 +116,7 @@ export class Context { }) } private findScope (key: string | number) { - if (isBlockedScopeKey(key)) return createScope() + if (isBlockedScopeKey(key) && this.ownPropertyOnly) return createScope() const hasKey = (obj: Scope) => { if (obj == null) return false return this.ownPropertyOnly @@ -147,7 +147,7 @@ export class Context { } export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean) { - if (isBlockedScopeKey(key)) return undefined + if (shouldBlockScopeKeyRead(obj, key, ownPropertyOnly)) 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 825d4b103..d0d77759e 100644 --- a/src/context/scope.ts +++ b/src/context/scope.ts @@ -13,6 +13,16 @@ export function isBlockedScopeKey (key: PropertyKey): boolean { return typeof key === 'string' && BLOCKED_SCOPE_KEYS.has(key) } +export function shouldBlockScopeKeyRead (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean): boolean { + if (!isBlockedScopeKey(key)) return false + if (ownPropertyOnly) return true + return !hasOwnProperty.call(obj, key) +} + +export function shouldBlockScopeKeyWrite (key: PropertyKey, ownPropertyOnly: boolean): boolean { + return ownPropertyOnly && isBlockedScopeKey(key) +} + export function createScope (from?: ScopeObject): ScopeObject { return from ? sanitizeScope(from) : Object.create(null) } @@ -20,7 +30,7 @@ export function createScope (from?: ScopeObject): ScopeObject { export function sanitizeScope (obj: ScopeObject): ScopeObject { const scope = Object.create(null) for (const key of Object.keys(obj)) { - if (!isBlockedScopeKey(key) && hasOwnProperty.call(obj, key)) { + if (hasOwnProperty.call(obj, key)) { scope[key] = obj[key] } } diff --git a/src/tags/assign.ts b/src/tags/assign.ts index 020563d69..8de8756d3 100644 --- a/src/tags/assign.ts +++ b/src/tags/assign.ts @@ -1,5 +1,5 @@ import { Value, Liquid, TopLevelToken, TagToken, Context, Tag } from '..' -import { isBlockedScopeKey } from '../context/scope' +import { shouldBlockScopeKeyWrite } from '../context/scope' import { Arguments } from '../template' import { IdentifierToken } from '../tokens' @@ -21,7 +21,7 @@ export default class extends Tag { this.value = new Value(this.tokenizer.readFilteredValue(), this.liquid) } * render (ctx: Context): Generator { - if (isBlockedScopeKey(this.key)) return + 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 cdf34051d..5ddacc54b 100644 --- a/src/tags/capture.ts +++ b/src/tags/capture.ts @@ -1,5 +1,5 @@ import { Liquid, Tag, Template, Context, TagToken, TopLevelToken } from '..' -import { isBlockedScopeKey } from '../context/scope' +import { shouldBlockScopeKeyWrite } from '../context/scope' import { Parser } from '../parser' import { IdentifierToken, QuotedToken } from '../tokens' import { isTagToken } from '../util' @@ -32,7 +32,7 @@ export default class extends Tag { * render (ctx: Context): Generator { const r = this.liquid.renderer const html = yield r.renderTemplates(this.templates, ctx) - if (isBlockedScopeKey(this.variable)) return + 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 a0ba81a21..f0caf8113 100644 --- a/src/tags/decrement.ts +++ b/src/tags/decrement.ts @@ -1,5 +1,5 @@ import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..' -import { isBlockedScopeKey } from '../context/scope' +import { shouldBlockScopeKeyWrite } from '../context/scope' import { IdentifierToken } from '../tokens' import { isNumber, stringify } from '../util' @@ -12,7 +12,7 @@ export default class extends Tag { this.variable = this.identifier.content } render (context: Context, emitter: Emitter) { - if (isBlockedScopeKey(this.variable)) return + 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 72da3711b..ad537e336 100644 --- a/src/tags/increment.ts +++ b/src/tags/increment.ts @@ -1,5 +1,5 @@ import { isNumber, stringify } from '../util' -import { isBlockedScopeKey } from '../context/scope' +import { shouldBlockScopeKeyWrite } from '../context/scope' import { Tag, Liquid, TopLevelToken, Emitter, TagToken, Context } from '..' import { IdentifierToken } from '../tokens' @@ -12,7 +12,7 @@ export default class extends Tag { this.variable = this.identifier.content } render (context: Context, emitter: Emitter) { - if (isBlockedScopeKey(this.variable)) return + 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 aa85511fb..404d5ce89 100644 --- a/test/integration/liquid/scope-security.spec.ts +++ b/test/integration/liquid/scope-security.spec.ts @@ -52,11 +52,26 @@ describe('scope security', function () { 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' } } } + it('should still block inherited __proto__ when ownPropertyOnly=false', async function () { + const scope = { foo: Object.create({ __proto__: { bar: 'BAR' } }) } await expect(liquid.parseAndRender('{{ foo.__proto__.bar }}', scope, { ownPropertyOnly: false })).resolves.toBe('') }) + it('should allow own __proto__ when ownPropertyOnly=false', async function () { + const scope = { foo: JSON.parse('{"__proto__": {"bar": "BAR"}}') } + await expect(liquid.parseAndRender('{{ foo.__proto__.bar }}', scope, { ownPropertyOnly: false })).resolves.toBe('BAR') + }) + + it('should allow own constructor when ownPropertyOnly=false', async function () { + const scope = { name: 'Alice', constructor: { name: 'Custom' } } + await expect(liquid.parseAndRender('{{ constructor.name }}', scope, { ownPropertyOnly: false })).resolves.toBe('Custom') + }) + + it('should still block inherited constructor when ownPropertyOnly=false', async function () { + const scope = { foo: {} } + await expect(liquid.parseAndRender('{{ foo.constructor.name }}', scope, { 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('')