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 }}')