From e743da0020d34e2ee547e1cc1a86b58377ebe1ce Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Tue, 7 Apr 2026 21:01:20 +0800 Subject: [PATCH] fix: sort and sort_natural filters bypass ownPropertyOnly (#869) Use _getFromScope for property access in sort/sort_natural filters to respect the ownPropertyOnly security option, preventing prototype chain traversal that could leak sensitive inherited properties. Also extract shared sortBy helper, add orderedCompare with nil handling consistent with caseInsensitiveCompare and Ruby Liquid. Made-with: Cursor --- src/context/scope.ts | 2 +- src/filters/array.ts | 26 +++++++----------- src/util/underscore.ts | 15 ++++++++--- test/integration/filters/array.spec.ts | 37 ++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/src/context/scope.ts b/src/context/scope.ts index f8f0ab057..9fcc06dae 100644 --- a/src/context/scope.ts +++ b/src/context/scope.ts @@ -1,6 +1,6 @@ import { Drop } from '../drop/drop' -interface ScopeObject extends Record { +interface ScopeObject extends Record { toLiquid?: () => any; } diff --git a/src/filters/array.ts b/src/filters/array.ts index 6792d572c..f22fcce20 100644 --- a/src/filters/array.ts +++ b/src/filters/array.ts @@ -1,4 +1,4 @@ -import { toArray, argumentsToValue, toValue, stringify, caseInsensitiveCompare, isArray, isNil, last as arrayLast, isArrayLike, toEnumerable } from '../util' +import { toArray, argumentsToValue, toValue, stringify, caseInsensitiveCompare, orderedCompare, isArray, isNil, last as arrayLast, isArrayLike, toEnumerable } from '../util' import { arrayIncludes, equals, evalToken, isTruthy } from '../render' import { Value, FilterImpl } from '../template' import { Tokenizer } from '../parser' @@ -20,8 +20,8 @@ export const reverse = argumentsToValue(function (this: FilterImpl, v: any[]) { return [...array].reverse() }) -export function * sort (this: FilterImpl, arr: T[], property?: string): IterableIterator { - const values: [T, string | number][] = [] +function * sortBy (this: FilterImpl, arr: T[], property: string | undefined, comparator: (a: unknown, b: unknown) => number): IterableIterator { + const values: [T, unknown][] = [] const array = toArray(arr) this.context.memoryLimit.use(array.length) for (const item of array) { @@ -30,21 +30,15 @@ export function * sort (this: FilterImpl, arr: T[], property?: string): Itera property ? yield this.context._getFromScope(item, stringify(property).split('.'), false) : item ]) } - return values.sort((lhs, rhs) => { - const lvalue = lhs[1] - const rvalue = rhs[1] - return lvalue < rvalue ? -1 : (lvalue > rvalue ? 1 : 0) - }).map(tuple => tuple[0]) + return values.sort((lhs, rhs) => comparator(lhs[1], rhs[1])).map(tuple => tuple[0]) } -export function sort_natural (this: FilterImpl, input: T[], property?: string) { - const propertyString = stringify(property) - const compare = property === undefined - ? caseInsensitiveCompare - : (lhs: T, rhs: T) => caseInsensitiveCompare(lhs[propertyString], rhs[propertyString]) - const array = toArray(input) - this.context.memoryLimit.use(array.length) - return [...array].sort(compare) +export function * sort (this: FilterImpl, arr: T[], property?: string): IterableIterator { + return yield * sortBy.call(this, arr, property, orderedCompare) +} + +export function * sort_natural (this: FilterImpl, arr: T[], property?: string): IterableIterator { + return yield * sortBy.call(this, arr, property, caseInsensitiveCompare) } export const size = (v: string | any[]) => (v && v.length) || 0 diff --git a/src/util/underscore.ts b/src/util/underscore.ts index ad829e0bc..f3558da1a 100644 --- a/src/util/underscore.ts +++ b/src/util/underscore.ts @@ -170,11 +170,20 @@ export function ellipsis (str: string, N: number): string { return str.length > N ? str.slice(0, N - 3) + '...' : str } +export function orderedCompare (a: any, b: any) { + if (isNil(a) && isNil(b)) return 0 + if (isNil(a)) return 1 + if (isNil(b)) return -1 + if (a < b) return -1 + if (a > b) return 1 + return 0 +} + // compare string in case-insensitive way, undefined values to the tail export function caseInsensitiveCompare (a: any, b: any) { - if (a == null && b == null) return 0 - if (a == null) return 1 - if (b == null) return -1 + if (isNil(a) && isNil(b)) return 0 + if (isNil(a)) return 1 + if (isNil(b)) return -1 a = toLowerCase.call(a) b = toLowerCase.call(b) if (a < b) return -1 diff --git a/test/integration/filters/array.spec.ts b/test/integration/filters/array.spec.ts index eb28db78d..b9212686f 100644 --- a/test/integration/filters/array.spec.ts +++ b/test/integration/filters/array.spec.ts @@ -315,6 +315,26 @@ describe('filters/array', function () { it('should return empty array for nil value', () => { return test('{{notDefined | sort | size}}', {}, '0') }) + it('should respect ownPropertyOnly', async () => { + const engine = new Liquid({ ownPropertyOnly: true }) + const a = Object.create({ secret: 'ccc' }) + a.name = 'a' + const b = Object.create({ secret: 'aaa' }) + b.name = 'b' + const html = await engine.parseAndRender( + '{{ arr | sort: "secret" | map: "name" | join: "," }}', + { arr: [a, b] } + ) + expect(html).toBe('a,b') + }) + it('should handle nil property values', async () => { + const arr = [{ age: 'cc' }, { name: 'x' }, { age: 'aa' }, { age: 'bb' }] + await test('{% assign sorted = arr | sort: "age" %}{% for item in sorted %}[{{ item.age }}]{% endfor %}', { arr }, '[aa][bb][cc][]') + }) + it('should handle mixed-type items', async () => { + const arr = ['40', null, 30, undefined, true, false, 0, 'str', 50] + await test('{% assign sorted = arr | sort %}{% for item in sorted %}[{{ item }}]{% endfor %}', { arr }, '[false][0][true][30][40][str][50][][]') + }) }) describe('sort_natural', function () { it('should sort alphabetically', () => { @@ -360,6 +380,23 @@ describe('filters/array', function () { { students: undefined }, '0' )) + it('should respect ownPropertyOnly', async () => { + const engine = new Liquid({ ownPropertyOnly: true }) + const target = Object.create({ secret: 'bbb' }) + const html = await engine.parseAndRender( + '{{ arr | sort_natural: "secret" | map: "secret" | join: "," }}', + { arr: [{ secret: 'ccc' }, target, { secret: 'aaa' }] } + ) + expect(html).toBe('aaa,ccc,') + }) + it('should handle nil property values', async () => { + const arr = [{ age: '40' }, { name: 'x' }, { age: 30 }, { age: 50 }] + await test('{% assign sorted = arr | sort_natural: "age" %}{% for item in sorted %}[{{ item.age }}]{% endfor %}', { arr }, '[30][40][50][]') + }) + it('should handle mixed-type items', async () => { + const arr = ['40', null, 30, undefined, true, false, 0, 'str', 50] + await test('{% assign sorted = arr | sort_natural %}{% for item in sorted %}[{{ item }}]{% endfor %}', { arr }, '[0][30][40][50][false][str][true][][]') + }) }) describe('uniq', function () { it('should uniq string list', function () {