From 552819a84b80c62306fe61072628a756272dc749 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Mon, 6 Jul 2026 20:12:09 +0800 Subject: [PATCH] fix: enforce ownPropertyOnly for inherited array indices (#924) * fix: enforce ownPropertyOnly for inherited array indices Route array index access (including negative indices, first/last, and the first/last filters) through a shared readArrayElement helper so that ownPropertyOnly hides prototype-inherited array indices, closing the GHSA-fwxr-j5w2-587m bypass. The option's scope (property/index access only, not filter transforms or iteration) is documented on the option. Co-authored-by: Cursor * fix(filters): invoke Array.prototype methods on unsanitized array values Call built-ins via Array.prototype..call(...) for values that come from scope (join, compact, concat, slice, where/reject) so an overridden instance method on unsanitized data cannot hijack filter behavior. Methods on freshly-created arrays are left as-is. Co-authored-by: Cursor * fix(filters): use String.prototype.slice for the string branch of slice Route the non-array branch through String.prototype.slice.call so the slice filter never dispatches through a possibly-overridden instance method, matching the Array.prototype guard. Co-authored-by: Cursor --------- Co-authored-by: Cursor --- src/context/context.spec.ts | 15 ++++++ src/context/context.ts | 20 +++---- src/filters/array.ts | 22 +++++--- src/liquid-options.ts | 5 +- src/util/underscore.ts | 6 +++ .../context/own-property-only.spec.ts | 54 +++++++++++++++++++ 6 files changed, 103 insertions(+), 19 deletions(-) create mode 100644 test/integration/context/own-property-only.spec.ts diff --git a/src/context/context.spec.ts b/src/context/context.spec.ts index f122174c9..d8eeac978 100644 --- a/src/context/context.spec.ts +++ b/src/context/context.spec.ts @@ -183,6 +183,21 @@ describe('Context', function () { ctx.push({ foo: Object.create({ bar: 'BAR' }) }) return expect(() => ctx.getSync(['foo', 'bar'])).toThrow(/undefined variable: foo.bar/) }) + it('should return undefined for inherited array indices', function () { + // eslint-disable-next-line no-extend-native + Array.prototype[0] = 'POLLUTED' + try { + const a: number[] = [] + a.length = 1 + ctx.push({ foo: a }) + expect(ctx.getSync(['foo', 0])).toEqual(undefined) + expect(ctx.getSync(['foo', -1])).toEqual(undefined) + expect(ctx.getSync(['foo', 'first'])).toEqual(undefined) + expect(ctx.getSync(['foo', 'last'])).toEqual(undefined) + } finally { + delete (Array.prototype as any)[0] + } + }) }) describe('.getAll()', function () { diff --git a/src/context/context.ts b/src/context/context.ts index b8951056f..9dcb711ea 100644 --- a/src/context/context.ts +++ b/src/context/context.ts @@ -3,7 +3,7 @@ import { Drop } from '../drop/drop' import { __assign } from 'tslib' import { NormalizedFullOptions, defaultOptions, RenderOptions } from '../liquid-options' import { createScope, Scope } from './scope' -import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, toLiquid, InternalUndefinedVariableError, toValueSync, isObject, Limiter, toValue } from '../util' +import { hasOwnProperty, isArray, isNil, isUndefined, isString, isFunction, isNumber, toLiquid, InternalUndefinedVariableError, toValueSync, isObject, Limiter, toValue, readArrayElement } from '../util' type PropertyKey = string | number; @@ -125,13 +125,13 @@ export class Context { obj = toLiquid(obj) key = toValue(key) as PropertyKey if (isNil(obj)) return obj - if (isArray(obj) && (key as number) < 0) return obj[obj.length + +key] + if (isArray(obj) && isNumber(key)) return readArrayElement(obj, key, this.ownPropertyOnly) const value = readJSProperty(obj, key, this.ownPropertyOnly) if (value === undefined && obj instanceof Drop) return obj.liquidMethodMissing(key, this) if (isFunction(value)) return value.call(obj) if (key === 'size') return readSize(obj) - else if (key === 'first') return readFirst(obj) - else if (key === 'last') return readLast(obj) + else if (key === 'first') return readFirst(obj, this.ownPropertyOnly) + else if (key === 'last') return readLast(obj, this.ownPropertyOnly) return value } } @@ -141,14 +141,14 @@ export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: b return obj[key] } -function readFirst (obj: Scope) { - if (isArray(obj)) return obj[0] - return obj['first'] +function readFirst (obj: Scope, ownPropertyOnly: boolean) { + if (isArray(obj)) return readArrayElement(obj, 0, ownPropertyOnly) + return readJSProperty(obj, 'first', ownPropertyOnly) } -function readLast (obj: Scope) { - if (isArray(obj)) return obj[obj.length - 1] - return obj['last'] +function readLast (obj: Scope, ownPropertyOnly: boolean) { + if (isArray(obj)) return readArrayElement(obj, -1, ownPropertyOnly) + return readJSProperty(obj, 'last', ownPropertyOnly) } function readSize (obj: Scope) { diff --git a/src/filters/array.ts b/src/filters/array.ts index 502af5355..e714d6588 100644 --- a/src/filters/array.ts +++ b/src/filters/array.ts @@ -1,4 +1,4 @@ -import { toArray, argumentsToValue, toValue, stringify, caseInsensitiveCompare, orderedCompare, isArray, isNil, last as arrayLast, isArrayLike, toEnumerable } from '../util' +import { toArray, argumentsToValue, toValue, stringify, caseInsensitiveCompare, orderedCompare, isArray, isNil, isArrayLike, readArrayElement, toEnumerable } from '../util' import { arrayIncludes, equals, evalToken, isTruthy } from '../render' import { Value, FilterImpl } from '../template' import { Tokenizer } from '../parser' @@ -10,10 +10,14 @@ export const join = argumentsToValue(function (this: FilterImpl, v: any[], arg: const sep = isNil(arg) ? ' ' : stringify(arg) const complexity = array.length * (1 + sep.length) this.context.memoryLimit.use(complexity) - return array.join(sep) + return Array.prototype.join.call(array, sep) +}) +export const last = argumentsToValue(function (this: FilterImpl, v: any) { + return isArrayLike(v) ? readArrayElement(v, -1, this.context.ownPropertyOnly) : '' +}) +export const first = argumentsToValue(function (this: FilterImpl, v: any) { + return isArrayLike(v) ? readArrayElement(v, 0, this.context.ownPropertyOnly) : '' }) -export const last = argumentsToValue((v: any) => isArrayLike(v) ? arrayLast(v) : '') -export const first = argumentsToValue((v: any) => isArrayLike(v) ? v[0] : '') export const reverse = argumentsToValue(function (this: FilterImpl, v: any[]) { const array = toArray(v) this.context.memoryLimit.use(array.length) @@ -66,14 +70,14 @@ export function * sum (this: FilterImpl, arr: Scope[], property?: string): Itera export function compact (this: FilterImpl, arr: T[]) { const array = toArray(arr) this.context.memoryLimit.use(array.length) - return array.filter(x => !isNil(toValue(x))) + return Array.prototype.filter.call(array, x => !isNil(toValue(x))) } export function concat (this: FilterImpl, v: T1[], arg: T2[] = []): (T1 | T2)[] { const lhs = toArray(v) const rhs = toArray(arg) this.context.memoryLimit.use(lhs.length + rhs.length) - return lhs.concat(rhs) + return Array.prototype.concat.call(lhs, rhs) } export function push (this: FilterImpl, v: T[], arg: T): T[] { @@ -110,7 +114,9 @@ export function slice (this: FilterImpl, v: T[] | string, begin: number, leng if (!isArray(v)) v = stringify(v) begin = begin < 0 ? v.length + begin : begin this.context.memoryLimit.use(length) - return v.slice(begin, begin + length) + return isArray(v) + ? Array.prototype.slice.call(v, begin, begin + length) + : String.prototype.slice.call(v, begin, begin + length) } function expectedMatcher (this: FilterImpl, expected: any): (v: any) => boolean { @@ -132,7 +138,7 @@ function * filter (this: FilterImpl, include: boolean, arr: T[ values.push(yield evalToken(token, this.context.spawn(item))) } const matcher = expectedMatcher.call(this, expected) - return arr.filter((_, i) => matcher(values[i]) === include) + return Array.prototype.filter.call(arr, (_, i) => matcher(values[i]) === include) } function * filter_exp (this: FilterImpl, include: boolean, arr: T[], itemName: string, exp: string): IterableIterator { diff --git a/src/liquid-options.ts b/src/liquid-options.ts index e6fc5d1f0..7ab1cbe2d 100644 --- a/src/liquid-options.ts +++ b/src/liquid-options.ts @@ -38,7 +38,10 @@ 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; - /** Hide scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates. */ + /** + * 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. + */ 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/src/util/underscore.ts b/src/util/underscore.ts index 665fdef6c..c60a93ebb 100644 --- a/src/util/underscore.ts +++ b/src/util/underscore.ts @@ -42,6 +42,12 @@ export function stringify (value: any): string { return String(value) } +export function readArrayElement (arr: any[], index: number, ownPropertyOnly: boolean) { + if (index < 0) index = arr.length + index + if (ownPropertyOnly && !hasOwnProperty.call(arr, index)) return undefined + return arr[index] +} + export function toEnumerable (val: any): T[] { val = toValue(val) if (isArray(val)) return val diff --git a/test/integration/context/own-property-only.spec.ts b/test/integration/context/own-property-only.spec.ts new file mode 100644 index 000000000..1d5c1dfc1 --- /dev/null +++ b/test/integration/context/own-property-only.spec.ts @@ -0,0 +1,54 @@ +import { Liquid } from '../../../src/liquid' + +describe('ownPropertyOnly / inherited array indices', function () { + const engine = new Liquid({ ownPropertyOnly: true }) + + function pollutedArrays () { + // eslint-disable-next-line no-extend-native + Array.prototype[0] = 'ARRAY_PROTO_POLLUTED' + ;(Object.prototype as any).secret = 'OBJECT_PROTO_POLLUTED' + const a: any[] = [] + a.length = 1 + const o = {} + return { + a, + o, + cleanup () { + delete (Array.prototype as any)[0] + delete (Object.prototype as any).secret + } + } + } + + const cases: [string, (ctx: ReturnType) => object, string][] = [ + ['{{ a[0] }}', ({ a }) => ({ a }), ''], + ['{{ a[-1] }}', ({ a }) => ({ a }), ''], + ['{{ o.secret }}', ({ o }) => ({ o }), ''], + ['{{ a.first }}', ({ a }) => ({ a }), ''], + ['{{ a.last }}', ({ a }) => ({ a }), ''], + ['{{ a | first }}', ({ a }) => ({ a }), ''], + ['{{ a | last }}', ({ a }) => ({ a }), ''], + ['{% assign x = a | first %}{{ x }}', ({ a }) => ({ a }), ''] + ] + + it.each(cases)('%s', function (src, scopeFn, expected) { + const ctx = pollutedArrays() + try { + expect(engine.parseAndRenderSync(src, scopeFn(ctx))).toBe(expected) + } finally { + ctx.cleanup() + } + }) + + it('still allows array length and size', function () { + const { a, cleanup } = pollutedArrays() + try { + expect(engine.parseAndRenderSync('{{ a.size }}', { a })).toBe('1') + const arr = [1, 2] + expect(engine.parseAndRenderSync('{{ arr | first }}', { arr })).toBe('1') + expect(engine.parseAndRenderSync('{{ arr[-1] }}', { arr })).toBe('2') + } finally { + cleanup() + } + }) +})