From cd94bba730ff3abeda01952cc932fb88e13f2156 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Sat, 20 Jun 2026 00:29:19 +0800 Subject: [PATCH] fix(security): respect ownPropertyOnly for size/first/last magic keys readSize, readFirst, and readLast bypassed the ownPropertyOnly guard by reading inherited properties from the prototype chain. Pass the flag through and only allow prototype size lookups when ownPropertyOnly is false. Co-authored-by: Cursor --- src/context/context.spec.ts | 42 +++++++++++++++++++++++++++++++++++++ src/context/context.ts | 18 +++++++++------- test/e2e/issues.spec.ts | 15 +++++++++++++ 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/context/context.spec.ts b/src/context/context.spec.ts index f122174c9..54ed4a696 100644 --- a/src/context/context.spec.ts +++ b/src/context/context.spec.ts @@ -183,6 +183,48 @@ describe('Context', function () { ctx.push({ foo: Object.create({ bar: 'BAR' }) }) return expect(() => ctx.getSync(['foo', 'bar'])).toThrow(/undefined variable: foo.bar/) }) + it('should block prototype size/first/last magic keys', function () { + Object.assign(Object.prototype, { size: 123, first: 'FIRST_PROTO', last: 'LAST_PROTO' }) + try { + ctx.push({ foo: {} }) + expect(ctx.getSync(['foo', 'size'])).toEqual(0) + expect(ctx.getSync(['foo', 'first'])).toEqual(undefined) + expect(ctx.getSync(['foo', 'last'])).toEqual(undefined) + } finally { + delete (Object.prototype as any).size + delete (Object.prototype as any).first + delete (Object.prototype as any).last + } + }) + it('should allow array first/last/size magic keys with ownPropertyOnly', function () { + ctx.push({ foo: [1, 2, 3] }) + expect(ctx.getSync(['foo', 'first'])).toEqual(1) + expect(ctx.getSync(['foo', 'last'])).toEqual(3) + expect(ctx.getSync(['foo', 'size'])).toEqual(3) + }) + it('should use prototype magic keys when ownPropertyOnly=false', function () { + Object.assign(Object.prototype, { size: 123, first: 'FIRST_PROTO', last: 'LAST_PROTO' }) + try { + ctx = new Context({}, { ownPropertyOnly: false } as any) + ctx.push({ foo: {} }) + expect(ctx.getSync(['foo', 'size'])).toEqual(123) + expect(ctx.getSync(['foo', 'first'])).toEqual('FIRST_PROTO') + expect(ctx.getSync(['foo', 'last'])).toEqual('LAST_PROTO') + } finally { + delete (Object.prototype as any).size + delete (Object.prototype as any).first + delete (Object.prototype as any).last + } + }) + it('should treat bracket access like dot access for magic keys', function () { + Object.assign(Object.prototype, { size: 123 }) + try { + ctx.push({ foo: {} }) + expect(ctx.getSync(['foo', 'size'])).toEqual(0) + } finally { + delete (Object.prototype as any).size + } + }) }) describe('.getAll()', function () { diff --git a/src/context/context.ts b/src/context/context.ts index b8951056f..850504454 100644 --- a/src/context/context.ts +++ b/src/context/context.ts @@ -129,9 +129,9 @@ export class Context { 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) + if (key === 'size') return readSize(obj, this.ownPropertyOnly) + else if (key === 'first') return readFirst(obj, this.ownPropertyOnly) + else if (key === 'last') return readLast(obj, this.ownPropertyOnly) return value } } @@ -141,18 +141,22 @@ export function readJSProperty (obj: Scope, key: PropertyKey, ownPropertyOnly: b return obj[key] } -function readFirst (obj: Scope) { +function readFirst (obj: Scope, ownPropertyOnly: boolean) { if (isArray(obj)) return obj[0] + if (ownPropertyOnly && !hasOwnProperty.call(obj, 'first') && !(obj instanceof Drop)) return undefined return obj['first'] } -function readLast (obj: Scope) { +function readLast (obj: Scope, ownPropertyOnly: boolean) { if (isArray(obj)) return obj[obj.length - 1] + if (ownPropertyOnly && !hasOwnProperty.call(obj, 'last') && !(obj instanceof Drop)) return undefined return obj['last'] } -function readSize (obj: Scope) { - if (hasOwnProperty.call(obj, 'size') || obj['size'] !== undefined) return obj['size'] +function readSize (obj: Scope, ownPropertyOnly: boolean) { + if (hasOwnProperty.call(obj, 'size')) return obj['size'] + if (!ownPropertyOnly && obj['size'] !== undefined) return obj['size'] if (isArray(obj) || isString(obj)) return obj.length + if (obj instanceof Set || obj instanceof Map) return obj.size if (typeof obj === 'object') return Object.keys(obj).length } diff --git a/test/e2e/issues.spec.ts b/test/e2e/issues.spec.ts index c5243fe66..d63396ab9 100644 --- a/test/e2e/issues.spec.ts +++ b/test/e2e/issues.spec.ts @@ -215,6 +215,21 @@ describe('Issues', function () { const html = engine.parseAndRenderSync('{{foo | size}}-{{bar.coo}}', { foo: 'foo', bar: Object.create({ coo: 'COO' }) }) expect(html).toBe('3-') }) + it('should block prototype pollution via size/first/last magic keys', () => { + Object.assign(Object.prototype, { size: 123, first: 'FIRST_PROTO', last: 'LAST_PROTO' }) + try { + const engine = new Liquid({ ownPropertyOnly: true }) + const tpl = '{{ foo.size }}-{{ foo.first }}-{{ foo.last }}-{{ foo["size"] }}' + expect(engine.parseAndRenderSync(tpl, { foo: {} })).toBe('0---0') + expect(engine.parseAndRenderSync('{{ arr.first }}-{{ arr.last }}-{{ arr.size }}', { arr: [1, 2, 3] })).toBe('1-3-3') + const engine2 = new Liquid({ ownPropertyOnly: false }) + expect(engine2.parseAndRenderSync('{{ foo.size }}-{{ foo.first }}-{{ foo.last }}', { foo: {} })).toBe('123-FIRST_PROTO-LAST_PROTO') + } finally { + delete (Object.prototype as any).size + delete (Object.prototype as any).first + delete (Object.prototype as any).last + } + }) it('Liquidjs divided_by not compatible with Ruby/Shopify Liquid #465', () => { const engine = new Liquid({ ownPropertyOnly: true }) const html = engine.parseAndRenderSync('{{ 5 | divided_by: 3, true }}')