mirror of
https://github.com/harttle/liquidjs.git
synced 2026-09-15 04:10:40 -07:00
fix: address scope-security review findings
Restore null-prototype hardening for Jekyll include bindings, colocate blocked-key checks with readJSProperty, align ownPropertyOnly JSDoc with security docs, and drop integration tests duplicated in context.spec. Co-authored-by: Cursor <[email protected]>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { Drop } from '../drop/drop'
|
||||
import { __assign } from 'tslib'
|
||||
import { NormalizedFullOptions, defaultOptions, RenderOptions } from '../liquid-options'
|
||||
import { createScope, Scope, shouldBlockScopeKeyRead } from './scope'
|
||||
import { createScope, Scope } from './scope'
|
||||
import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, isNumber, toLiquid, InternalUndefinedVariableError, toValueSync, isObject, Limiter, toValue, readArrayElement } from '../util'
|
||||
|
||||
type PropertyKey = string | number;
|
||||
@@ -165,6 +165,14 @@ export class Context {
|
||||
}
|
||||
}
|
||||
|
||||
const BLOCKED_SCOPE_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
|
||||
|
||||
function shouldBlockScopeKeyRead (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean): boolean {
|
||||
if (typeof key !== 'string' || !BLOCKED_SCOPE_KEYS.has(key)) return false
|
||||
if (ownPropertyOnly) return true
|
||||
return !hasOwnProperty.call(obj, key)
|
||||
}
|
||||
|
||||
export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: boolean) {
|
||||
if (shouldBlockScopeKeyRead(obj, key, ownPropertyOnly)) return undefined
|
||||
if (ownPropertyOnly && !hasOwnProperty.call(obj, key) && !(obj instanceof Drop)) return undefined
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { Drop } from '../drop/drop'
|
||||
import { hasOwnProperty } from '../util'
|
||||
|
||||
export interface ScopeObject extends Record<string | number | symbol, any> {
|
||||
toLiquid?: () => any;
|
||||
@@ -7,18 +6,6 @@ export interface ScopeObject extends Record<string | number | symbol, any> {
|
||||
|
||||
export type Scope = ScopeObject | Drop
|
||||
|
||||
const BLOCKED_SCOPE_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
|
||||
|
||||
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 createScope (from?: ScopeObject): ScopeObject {
|
||||
return Object.assign(Object.create(null), from)
|
||||
}
|
||||
|
||||
@@ -39,8 +39,9 @@ export interface LiquidOptions {
|
||||
/** Catch all errors instead of exit upon one. Please note that render errors won't be reached when parse fails. */
|
||||
catchAllErrors?: boolean;
|
||||
/**
|
||||
* Hide scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates.
|
||||
* This only applies to property/index access on scope objects. Filter transforms and iteration operate on the resolved value with standard JavaScript semantics, so prototype-inherited array indices may still be surfaced by them.
|
||||
* Limit template property reads on plain scope objects to own properties (no inherited prototype keys). Defaults to `true`.
|
||||
* Proto-related keys (`__proto__`, `constructor`, `prototype`) are blocked when `true` (even as own properties); when `false`, own properties with those names are allowed and inherited prototype-chain access to those names is still blocked.
|
||||
* Drops, iteration, `.size`/`.first`/`.last`, filters, and custom tags follow separate rules.
|
||||
*/
|
||||
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`. **/
|
||||
|
||||
+2
-2
@@ -1,5 +1,5 @@
|
||||
import { Template, ValueToken, TopLevelToken, Liquid, Tag, assert, evalToken, Hash, Emitter, TagToken, Context } from '..'
|
||||
import { BlockMode, Scope } from '../context'
|
||||
import { BlockMode, Scope, createScope } from '../context'
|
||||
import { Parser } from '../parser'
|
||||
import { Argument, Arguments, PartialScope } from '../template'
|
||||
import { isString, isValueToken } from '../util'
|
||||
@@ -40,7 +40,7 @@ export default class extends Tag {
|
||||
const scope = (yield hash.render(ctx)) as Scope
|
||||
if (withVar) scope[filepath] = yield evalToken(withVar, ctx)
|
||||
const templates = (yield liquid._parsePartialFile(filepath, ctx.sync, this.currentFile)) as Template[]
|
||||
ctx.push(ctx.opts.jekyllInclude ? { include: scope } : scope)
|
||||
ctx.push(ctx.opts.jekyllInclude ? { include: createScope(scope) } : scope)
|
||||
yield renderer.renderTemplates(templates, ctx, emitter)
|
||||
ctx.pop()
|
||||
ctx.restoreRegister(saved)
|
||||
|
||||
@@ -8,22 +8,6 @@ describe('scope security', 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 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 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' }
|
||||
|
||||
Reference in New Issue
Block a user