mirror of
https://github.com/harttle/liquidjs.git
synced 2026-09-18 05:50:43 -07:00
fix(security): block Object.prototype filter/tag lookups (RCE)
`liquid.filters` and `liquid.tags` were plain `{}` so bracket access on
template-controlled keys inherited from `Object.prototype`. Most damaging:
`{{ x | valueOf }}` resolved to `Object.prototype.valueOf`, which the
filter pipeline called as a handler with `this = FilterImpl`; valueOf
returns its receiver, leaking `context`, `liquid`, `token` (and via them
parser, loader, fs) into the template — chain that with `group_by`/`where`
gadgets and an attacker reaches `Function`/`child_process` for RCE.
Same shape on the tag side: `{% constructor %}` bypassed the
"tag not found" assertion and crashed with a confusing message.
Use null-prototype storage so `liquid.filters[name]` / `liquid.tags[name]`
only resolve to explicitly registered entries. The existing
`assert(impl || !strictFilters)` and `assert(TagClass, ...)` now do the
right thing for `valueOf`, `toString`, `constructor`, `__proto__`,
`hasOwnProperty`, `isPrototypeOf`, `__defineGetter__`, etc.
Co-authored-by: Cursor <[email protected]>
This commit is contained in:
+2
-2
@@ -15,8 +15,8 @@ export class Liquid {
|
||||
* @deprecated will be removed. In tags use `this.parser` instead
|
||||
*/
|
||||
public readonly parser: Parser
|
||||
public readonly filters: Record<string, FilterImplOptions> = {}
|
||||
public readonly tags: Record<string, TagClass> = {}
|
||||
public readonly filters: Record<string, FilterImplOptions> = Object.create(null)
|
||||
public readonly tags: Record<string, TagClass> = Object.create(null)
|
||||
|
||||
public constructor (opts: LiquidOptions = {}) {
|
||||
this.options = normalize(opts)
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
import { Liquid } from '../../../src/liquid'
|
||||
|
||||
describe('security', () => {
|
||||
describe('Object.prototype filter names', () => {
|
||||
// Regression: `{{ 1 | valueOf }}` used to resolve to Object.prototype.valueOf
|
||||
// and, when invoked as a filter handler, return the FilterImpl `this` — leaking
|
||||
// `context`, `liquid`, options, the parser, the loader, etc., enabling RCE.
|
||||
it('should treat valueOf as an unregistered filter (identity)', async () => {
|
||||
const liquid = new Liquid()
|
||||
const out = await liquid.parseAndRender('{% assign r = 1 | valueOf %}{{ r.liquid.options.fs.sep }}|{{ r }}')
|
||||
expect(out).toBe('|1')
|
||||
})
|
||||
it('should not leak FilterImpl via valueOf', async () => {
|
||||
const liquid = new Liquid()
|
||||
const out = await liquid.parseAndRender('{% assign r = 1 | valueOf %}{{ r.context }}/{{ r.liquid }}/{{ r.token }}')
|
||||
expect(out).toBe('//')
|
||||
})
|
||||
it.each(['toString', 'constructor', 'hasOwnProperty', 'isPrototypeOf', '__proto__', '__defineGetter__'])(
|
||||
'should treat %s as an unregistered filter',
|
||||
async (name) => {
|
||||
const liquid = new Liquid()
|
||||
const out = await liquid.parseAndRender(`{{ "x" | ${name} }}`)
|
||||
expect(out).toBe('x')
|
||||
}
|
||||
)
|
||||
it('should throw under strictFilters for inherited method names', async () => {
|
||||
const liquid = new Liquid({ strictFilters: true })
|
||||
await expect(liquid.parseAndRender('{{ 1 | valueOf }}')).rejects.toThrow('undefined filter: valueOf')
|
||||
})
|
||||
})
|
||||
describe('Object.prototype tag names', () => {
|
||||
// Regression: `{% constructor %}` used to resolve to Object via tags['constructor'],
|
||||
// bypassing the "tag not found" assertion and crashing later with a confusing error.
|
||||
it.each(['constructor', 'toString', 'valueOf', 'hasOwnProperty', '__proto__'])(
|
||||
'should report %s as unknown tag',
|
||||
(name) => {
|
||||
const liquid = new Liquid()
|
||||
expect(() => liquid.parse(`{% ${name} %}`)).toThrow(`tag "${name}" not found`)
|
||||
}
|
||||
)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user