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 <[email protected]>

* fix(filters): invoke Array.prototype methods on unsanitized array values

Call built-ins via Array.prototype.<m>.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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Cursor <[email protected]>

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 () {

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) {

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<T> (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<T1, T2> (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<T> (this: FilterImpl, v: T[], arg: T): T[] {
@@ -110,7 +114,9 @@ export function slice<T> (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<T extends object> (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<T extends object> (this: FilterImpl, include: boolean, arr: T[], itemName: string, exp: string): IterableIterator<unknown> {

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;

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<T = unknown> (val: any): T[] {
   val = toValue(val)
   if (isArray(val)) return val

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<typeof pollutedArrays>) => 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()
+    }
+  })
+})