diff --git a/packages/@n8n/expression-runtime/src/runtime/jmespath.ts b/packages/@n8n/expression-runtime/src/runtime/jmespath.ts index d7652bfa107..0c726f4206b 100644 --- a/packages/@n8n/expression-runtime/src/runtime/jmespath.ts +++ b/packages/@n8n/expression-runtime/src/runtime/jmespath.ts @@ -2,6 +2,37 @@ import * as jmespath from 'jmespath'; import { ExpressionError } from './safe-globals'; +const unsafeJmespathProperties = [ + '__proto__', + 'prototype', + 'constructor', + 'getPrototypeOf', + 'setPrototypeOf', + 'getOwnPropertyDescriptor', + 'getOwnPropertyDescriptors', + 'defineProperty', + 'defineProperties', + 'mainModule', + 'binding', + '_linkedBinding', + '_load', + 'prepareStackTrace', + '__lookupGetter__', + '__lookupSetter__', + '__defineGetter__', + '__defineSetter__', + 'caller', + 'arguments', + 'getBuiltinModule', + 'dlopen', + 'execve', + 'loadEnvFile', +]; + +const unsafeJmespathPropertyPattern = new RegExp( + `\\b(?:${unsafeJmespathProperties.map((p) => p.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|')})\\b`, +); + /** * In-isolate `$jmespath` / `$jmesPath` implementation. * @@ -15,6 +46,7 @@ import { ExpressionError } from './safe-globals'; * * Behavioural parity with the host wrapper: * - Throws `ExpressionError` (same name) when args are wrong. + * - Rejects queries that contain unsafe property tokens. * - Spreads non-array, non-null objects to drop proxies at the top level. * * Note on lazy proxies: when `data` is a lazy proxy (e.g. `$json`), each @@ -28,6 +60,15 @@ export function jmesPath(data: unknown, query: string): unknown { throw new ExpressionError('expected two arguments (Object, string) for this function'); } + // jmespath decodes escape sequences inside quoted identifiers, so the + // token check must run against an unescaped query. Reject any backslash + // up front to keep the property-name match meaningful. + if (query.includes('\\') || unsafeJmespathPropertyPattern.test(query)) { + throw new ExpressionError( + 'Cannot access this property in a jmespath query due to security concerns', + ); + } + if (data !== null && !Array.isArray(data) && typeof data === 'object') { return jmespath.search({ ...(data as Record) }, query); } diff --git a/packages/workflow/test/expression.test.ts b/packages/workflow/test/expression.test.ts index debd62369fa..9f3a9bcf7b1 100644 --- a/packages/workflow/test/expression.test.ts +++ b/packages/workflow/test/expression.test.ts @@ -470,46 +470,28 @@ describe('Expression', () => { ); }); - // The $jmespath restricted-identifier defense differs by engine: - // legacy → the host wrapper (workflow-data-proxy.ts) throws - // "due to security concerns" via an explicit token denylist. - // vm → jmespath runs in-isolate with no denylist, but a restricted - // identifier can never leak a usable constructor/prototype to - // the host: `constructor` resolves to the Object constructor, - // which fails to serialize across the isolate boundary (throws), - // while `getPrototypeOf`/`prototype`/`__proto__` are key-misses - // or non-cloneable internals → the result is never a callable. - // (Structural isolation rather than a denylist — see - // packages/@n8n/expression-runtime/src/runtime/jmespath.ts.) - const expectJmespathBlocked = (expr: string) => { - if (process.env.N8N_EXPRESSION_ENGINE === 'vm') { - let result: unknown; - let threw = false; - try { - result = evaluate(expr); - } catch { - threw = true; - } - expect(threw || typeof result !== 'function').toBe(true); - } else { - expect(() => evaluate(expr)).toThrow(/due to security concerns/); - } - }; - it('should reject jmespath queries that reference restricted identifiers', () => { - expectJmespathBlocked('={{ $jmespath({a:1}, "constructor") }}'); - expectJmespathBlocked('={{ $jmespath({a:1}, "__proto__") }}'); - expectJmespathBlocked('={{ $jmespath({a:1}, "prototype") }}'); + expect(() => evaluate('={{ $jmespath({a:1}, "constructor") }}')).toThrow( + /due to security concerns/, + ); + expect(() => evaluate('={{ $jmespath({a:1}, "__proto__") }}')).toThrow( + /due to security concerns/, + ); + expect(() => evaluate('={{ $jmespath({a:1}, "prototype") }}')).toThrow( + /due to security concerns/, + ); }); it('should reject jmespath queries that reference restricted identifiers (alias)', () => { - expectJmespathBlocked('={{ $jmesPath({a:1}, "getPrototypeOf") }}'); + expect(() => evaluate('={{ $jmesPath({a:1}, "getPrototypeOf") }}')).toThrow( + /due to security concerns/, + ); }); it('should reject computed-string jmespath queries built from restricted identifiers', () => { - expectJmespathBlocked( - '={{ $jmespath({a:1}, String.fromCharCode(99,111,110,115,116,114,117,99,116,111,114)) }}', - ); + const payload = + '={{ $jmespath({a:1}, String.fromCharCode(99,111,110,115,116,114,117,99,116,111,114)) }}'; + expect(() => evaluate(payload)).toThrow(/due to security concerns/); }); it('should still allow jmespath queries that contain restricted names as substrings', () => {