diff --git a/packages/js-runtime/src/execute.ts b/packages/js-runtime/src/execute.ts index a17918f15..48fada97e 100644 --- a/packages/js-runtime/src/execute.ts +++ b/packages/js-runtime/src/execute.ts @@ -1,3 +1,5 @@ +import { validate } from './validate'; + /** * Executes a JavaScript function template * @param code - JavaScript function code (arrow function or function expression) @@ -8,6 +10,14 @@ export function execute( code: string, payload: Record, ): unknown { + // Templates are checked when they are saved, but the stored string is what + // ends up in new Function() here. Check it again at run time rather than + // trusting whatever passed validation at save time. + const validation = validate(code); + if (!validation.valid) { + throw new Error(`Invalid JavaScript template: ${validation.error}`); + } + try { // Create the function code that will be executed // 'use strict' ensures 'this' is undefined (not global object) diff --git a/packages/js-runtime/src/validate.test.ts b/packages/js-runtime/src/validate.test.ts index f7278af55..2a4bb8cc7 100644 --- a/packages/js-runtime/src/validate.test.ts +++ b/packages/js-runtime/src/validate.test.ts @@ -139,6 +139,101 @@ describe('validate', () => { }); }); + describe('Computed and indirect access', () => { + it('should block a computed call target', () => { + const result = validate( + `(payload) => payload['constructor']['constructor']('return 1')()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it('should block a computed call target reached by optional chaining', () => { + const result = validate( + `(payload) => payload?.['constructor']['constructor']('return 1')()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it('should block a computed key written with escape sequences', () => { + const result = validate( + `(payload) => payload['\\u0063onstructor']['constructor']('return 1')()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it('should block a computed call target on a nested value', () => { + const result = validate( + `(payload) => payload.properties['toString']()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it("should block 'new' on a non-identifier callee", () => { + const result = validate( + `(payload) => new (payload['constructor']['constructor'])('return 1')`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain("target of 'new'"); + }); + + it("should block 'new' on a member expression", () => { + const result = validate('(payload) => new payload.Thing()'); + expect(result.valid).toBe(false); + expect(result.error).toContain("target of 'new'"); + }); + + it('should block dynamic import()', () => { + const result = validate(`(payload) => import('node:fs')`); + expect(result.valid).toBe(false); + expect(result.error).toContain('Dynamic import()'); + }); + }); + + describe('Prototype writes', () => { + it('should block writing through __proto__', () => { + const result = validate( + '(payload) => { payload.__proto__.x = 1; return payload; }', + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('__proto__'); + }); + + it('should block writing through a computed __proto__ key', () => { + const result = validate( + `(payload) => { payload['__proto__'].x = 1; return payload; }`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('__proto__'); + }); + + it('should block writing to a prototype', () => { + const result = validate( + '(payload) => { payload.constructor.prototype.x = 1; return payload; }', + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('not allowed'); + }); + + it('should block writing through a no-substitution template literal key', () => { + const result = validate( + '(payload) => { payload[`__proto__`].x = 1; return payload; }', + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('__proto__'); + }); + + it('should still allow writing an ordinary property', () => { + const result = validate( + '(payload) => { const out = {}; out.event = payload.name; return out; }', + ); + expect(result.valid).toBe(true); + }); + }); + describe('Invalid syntax', () => { it('should reject non-function code', () => { const result = validate('const x = 1;'); @@ -328,5 +423,14 @@ describe('execute', () => { execute(code, basePayload); }).toThrow('Error executing JavaScript template'); }); + + it('should refuse to run a stored template that fails validation', () => { + // A template that was persisted at some point but does not pass + // validation must not reach new Function(). + const code = `(payload) => payload['constructor']['constructor']('return 1')()`; + expect(() => { + execute(code, basePayload); + }).toThrow('Invalid JavaScript template'); + }); }); }); diff --git a/packages/js-runtime/src/validate.ts b/packages/js-runtime/src/validate.ts index c52a523a9..5e1e96208 100644 --- a/packages/js-runtime/src/validate.ts +++ b/packages/js-runtime/src/validate.ts @@ -11,6 +11,74 @@ import { walkNode, } from './ast-walker'; +/** + * Property names that must never be written through. Assigning to any of + * these reaches objects shared with the rest of the worker process. + */ +const FORBIDDEN_WRITE_PROPERTIES = new Set([ + '__proto__', + 'constructor', + 'prototype', +]); + +/** + * The static name of a member expression's property, or undefined when the key + * is only known at run time (obj[someVariable]). + */ +function staticPropertyName( + member: Record +): string | undefined { + const prop = member.property as Record | undefined; + if (!prop) { + return undefined; + } + if (member.computed) { + // A literal key can be resolved. Babel has already decoded any \u / \x + // escapes into StringLiteral.value by this point. A template literal + // with no substitutions (`__proto__`) is just as static as a string + // literal and must resolve the same way, or it walks past this check + // unseen. + if (prop.type === 'StringLiteral') { + return prop.value as string; + } + if (prop.type === 'TemplateLiteral') { + const expressions = prop.expressions as unknown[]; + const quasis = prop.quasis as Record[]; + if (expressions.length === 0 && quasis.length === 1) { + const value = quasis[0]!.value as Record; + return value.cooked as string; + } + } + return undefined; + } + return prop.type === 'Identifier' ? (prop.name as string) : undefined; +} + +/** + * Walk an assignment target back down its member chain and return the first + * forbidden property name it passes through, if any. payload.__proto__.x is a + * write to 'x' but goes through '__proto__', so the whole chain matters. + */ +function forbiddenPropertyInChain( + target: Record +): string | undefined { + let current: Record | undefined = target; + + while ( + current && + (current.type === 'MemberExpression' || + current.type === 'OptionalMemberExpression') + ) { + const name = staticPropertyName(current); + if (name && FORBIDDEN_WRITE_PROPERTIES.has(name)) { + return name; + } + current = current.object as Record | undefined; + } + + return undefined; +} + /** * Validates that a JavaScript function is safe to execute * by checking the AST for allowed operations only (allowlist approach) @@ -107,6 +175,13 @@ export function validate(code: string): { return; } + // Block dynamic import(). @babel/parser emits 'Import' as the callee of + // the surrounding CallExpression; 'ImportExpression' is the ESTree shape. + if (node.type === 'Import' || node.type === 'ImportExpression') { + validationError = 'Dynamic import() is not allowed'; + return; + } + // Block function declarations inside the function body // (FunctionDeclaration creates a named function, not allowed) if (node.type === 'FunctionDeclaration') { @@ -212,12 +287,17 @@ export function validate(code: string): { const prop = callee.property as Record; const computed = callee.computed as boolean; + // A computed key (obj[expr]()) cannot be matched against the + // allowlist, because the property name is only known at run time. + // Refuse it rather than letting it past the checks below unseen. + if (computed) { + validationError = + 'Computed property access on a call target is not allowed. Use a literal method name, e.g. value.toUpperCase().'; + return; + } + // Static method call on global object: Math.random(), JSON.parse() - if ( - obj.type === 'Identifier' && - prop.type === 'Identifier' && - !computed - ) { + if (obj.type === 'Identifier' && prop.type === 'Identifier') { const objName = obj.name as string; const methodName = prop.name as string; @@ -232,7 +312,7 @@ export function validate(code: string): { // Instance method call: arr.map(), str.toLowerCase(), arr?.map() // We allow these if the method name is in ALLOWED_INSTANCE_METHODS - if (prop.type === 'Identifier' && !computed) { + if (prop.type === 'Identifier') { const methodName = prop.name as string; // If calling on something other than an allowed global, @@ -253,12 +333,35 @@ export function validate(code: string): { // Check 'new' expressions - only allow new Date() if (node.type === 'NewExpression') { const callee = node.callee as Record; - if (callee.type === 'Identifier') { - const name = callee.name as string; - if (name !== 'Date') { - validationError = `'new ${name}()' is not allowed. Only 'new Date()' is permitted.`; - return; - } + + // Anything other than a bare identifier (a member expression, a + // parenthesised expression, another call) names a constructor we + // cannot resolve, so it can never be the Date we allow. + if (callee.type !== 'Identifier') { + validationError = + "The target of 'new' must be a plain identifier. Only 'new Date()' is permitted."; + return; + } + + const name = callee.name as string; + if (name !== 'Date') { + validationError = `'new ${name}()' is not allowed. Only 'new Date()' is permitted.`; + return; + } + } + + // Block writes that reach the prototype chain: payload.__proto__.x = 1, + // payload['constructor'].prototype.y = 2. Reading these is already + // handled by the call and 'new' checks above. + if ( + node.type === 'AssignmentExpression' || + node.type === 'UpdateExpression' + ) { + const target = (node.left ?? node.argument) as Record; + const reached = forbiddenPropertyInChain(target); + if (reached) { + validationError = `Assigning through '${reached}' is not allowed.`; + return; } } });