diff --git a/BREAKING_CHANGES.md b/BREAKING_CHANGES.md new file mode 100644 index 00000000..c94c45c4 --- /dev/null +++ b/BREAKING_CHANGES.md @@ -0,0 +1,87 @@ +# Breaking Changes + +This document lists breaking changes in the library to help users migrate between versions. + +## Version 5.0.0 + +### Security: Functions Must Be Registered Explicitly + +**Background**: This change addresses critical security vulnerabilities: +- [CVE-2025-12735](https://github.com/advisories/GHSA-jc85-fpwf-qm7x) - Code injection via arbitrary function calls +- [CVE-2025-13204](https://github.com/advisories) - Prototype pollution via `__proto__`, `prototype`, `constructor` access +- [silentmatt/expr-eval#289](https://github.com/silentmatt/expr-eval/issues/289) - Member function call bypass + +**What Changed**: Functions can no longer be passed directly via the evaluation context. All functions that need to be called from expressions must be explicitly registered in `parser.functions`. + +**Before (Vulnerable)**: +```typescript +const parser = new Parser(); + +// This pattern is NO LONGER ALLOWED +parser.evaluate('customFunc()', { customFunc: () => 'result' }); + +// This also NO LONGER WORKS +parser.evaluate('obj.method()', { + obj: { + method: () => 'dangerous' + } +}); +``` + +**After (Secure)**: +```typescript +const parser = new Parser(); + +// Register functions explicitly +parser.functions.customFunc = () => 'result'; +parser.evaluate('customFunc()'); + +// For methods on objects, register them as top-level functions +parser.functions.objMethod = () => 'safe'; +parser.evaluate('objMethod()'); +``` + +**What Still Works**: +- Passing primitive values (strings, numbers, booleans) via context +- Passing arrays and objects with non-function properties via context +- Using built-in Math functions (sin, cos, sqrt, etc.) +- Using inline-defined functions in expressions: `(f(x) = x * 2)(5)` +- Using functions registered in `parser.functions` + +**Migration Guide**: + +1. **Identify function usage**: Search your codebase for patterns like `evaluate('...', { fn: ... })` where `fn` is a function. + +2. **Register functions before evaluation**: + ```typescript + // Before + parser.evaluate('calculate(x)', { calculate: myFunc, x: 5 }); + + // After + parser.functions.calculate = myFunc; + parser.evaluate('calculate(x)', { x: 5 }); + ``` + +3. **For dynamic functions**: If you need to register functions dynamically: + ```typescript + const parser = new Parser(); + parser.functions.dynamicFn = createDynamicFunction(); + const result = parser.evaluate('dynamicFn()'); + delete parser.functions.dynamicFn; // Clean up if needed + ``` + +### Protected Properties + +Access to the following properties is now blocked to prevent prototype pollution attacks: +- `__proto__` +- `prototype` +- `constructor` + +Attempting to access these properties in variable names or member expressions will throw an `AccessError`. + +**Example**: +```typescript +// These will throw AccessError +parser.evaluate('x.__proto__', { x: {} }); +parser.evaluate('__proto__', { __proto__: {} }); +``` diff --git a/package-lock.json b/package-lock.json index b464394e..4dde4626 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@pro-fa/expr-eval", - "version": "4.2.0", + "version": "5.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@pro-fa/expr-eval", - "version": "4.2.0", + "version": "5.0.0", "license": "MIT", "dependencies": { "vscode-languageserver-textdocument": "^1.0.12" diff --git a/package.json b/package.json index cc5f2321..280940e1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pro-fa/expr-eval", - "version": "4.2.0", + "version": "5.0.0", "description": "Mathematical expression evaluator", "keywords": [ "expression", diff --git a/src/core/evaluate.ts b/src/core/evaluate.ts index fb47de43..5e9f3640 100644 --- a/src/core/evaluate.ts +++ b/src/core/evaluate.ts @@ -17,6 +17,12 @@ import { ExpressionValidator } from '../validation/expression-validator.js'; // cSpell:words IOBJECT IOBJECTEND // cSpell:words nstack +/** + * Counter for generating unique keys for inline-defined functions. + * This prevents collision attacks by using a monotonically increasing counter. + */ +let inlineFunctionCounter = 0; + /** * Wrapper for lazy expression evaluation * Used for short-circuit evaluation of logical operators and conditionals @@ -170,6 +176,8 @@ function evaluateExpressionToken(expr: Expression, values: EvaluationValues, tok let valueResolved = false; if (variableName in values) { const variableValue = values[variableName]; + // Security: Validate that functions from context are allowed before pushing onto stack + ExpressionValidator.validateAllowedFunction(variableValue, expr.functions, expr.toString()); nstack.push(variableValue); valueResolved = true; } else { @@ -184,13 +192,19 @@ function evaluateExpressionToken(expr: Expression, values: EvaluationValues, tok // The parser's resolver function returned { alias: "xxx" }, we want to use // resolved.alias in place of token.value. if (resolvedVariable.alias in values) { - nstack.push(values[resolvedVariable.alias]); + const aliasValue = values[resolvedVariable.alias]; + // Security: Validate that functions from context are allowed + ExpressionValidator.validateAllowedFunction(aliasValue, expr.functions, expr.toString()); + nstack.push(aliasValue); valueResolved = true; } } else if (typeof resolvedVariable === 'object' && resolvedVariable && 'value' in resolvedVariable) { // The parser's resolver function returned { value: }, use // as the value of the token. - nstack.push(resolvedVariable.value); + const resolvedValue = resolvedVariable.value; + // Security: Validate that functions from context are allowed + ExpressionValidator.validateAllowedFunction(resolvedValue, expr.functions, expr.toString()); + nstack.push(resolvedValue); valueResolved = true; } } @@ -215,6 +229,8 @@ function evaluateExpressionToken(expr: Expression, values: EvaluationValues, tok } const functionToCall = nstack.pop(); ExpressionValidator.validateFunctionCall(functionToCall, String(functionToCall), expr.toString()); + // Security: Validate the function is allowed before calling it + ExpressionValidator.validateAllowedFunction(functionToCall, expr.functions, expr.toString()); nstack.push(functionToCall.apply(undefined, functionArgs)); } else if (type === IFUNDEF) { // Create closure to keep references to arguments and expression @@ -238,6 +254,10 @@ function evaluateExpressionToken(expr: Expression, values: EvaluationValues, tok value: functionName, writable: false }); + // Security: Register the inline-defined function as allowed using a unique counter-based key + // This prevents collision attacks since the key cannot be predicted or controlled by user input + const uniqueKey = `__inline_fn_${inlineFunctionCounter++}__`; + expr.functions[uniqueKey] = userDefinedFunction; values[functionName] = userDefinedFunction; return userDefinedFunction; })()); @@ -247,7 +267,13 @@ function evaluateExpressionToken(expr: Expression, values: EvaluationValues, tok nstack.push(token); } else if (type === IMEMBER) { const memberParent = nstack.pop(); - nstack.push(memberParent === undefined || token === undefined || token.value === undefined ? undefined : memberParent[token.value]); + const propertyName = token.value as string; + // Security: Block access to dangerous prototype properties + ExpressionValidator.validateMemberAccess(propertyName, expr.toString()); + const memberValue = memberParent === undefined || token === undefined || token.value === undefined ? undefined : memberParent[propertyName]; + // Security: Validate that member functions are allowed before pushing onto stack + ExpressionValidator.validateAllowedFunction(memberValue, expr.functions, expr.toString()); + nstack.push(memberValue); } else if (type === IENDSTATEMENT) { nstack.pop(); } else if (type === IARRAY) { diff --git a/src/validation/expression-validator.ts b/src/validation/expression-validator.ts index 98206a62..af42e7dd 100644 --- a/src/validation/expression-validator.ts +++ b/src/validation/expression-validator.ts @@ -7,6 +7,54 @@ */ import { AccessError, FunctionError } from '../types/errors.js'; +import type { OperatorFunction } from '../types/index.js'; + +/** + * Set of dangerous property names that could lead to prototype pollution + */ +const DANGEROUS_PROPERTIES = new Set(['__proto__', 'prototype', 'constructor']); + +/** + * Safe Math functions that are allowed by default. + * These are immutable references to the standard Math object methods. + */ +const SAFE_MATH_FUNCTIONS: ReadonlySet = new Set([ + Math.abs, + Math.acos, + Math.asin, + Math.atan, + Math.atan2, + Math.ceil, + Math.cos, + Math.exp, + Math.floor, + Math.log, + Math.max, + Math.min, + Math.pow, + Math.random, + Math.round, + Math.sin, + Math.sqrt, + Math.tan, + Math.log10, + Math.log2, + Math.log1p, + Math.expm1, + Math.cosh, + Math.sinh, + Math.tanh, + Math.acosh, + Math.asinh, + Math.atanh, + Math.hypot, + Math.trunc, + Math.sign, + Math.cbrt, + Math.clz32, + Math.imul, + Math.fround +]); /** * Validation utilities for expression evaluation @@ -16,7 +64,7 @@ export class ExpressionValidator { * Validates variable name to prevent prototype pollution */ static validateVariableName(variableName: string, expressionString: string): void { - if (/^__proto__|prototype|constructor$/.test(variableName)) { + if (DANGEROUS_PROPERTIES.has(variableName)) { throw new AccessError( 'Prototype access detected', { @@ -27,6 +75,78 @@ export class ExpressionValidator { } } + /** + * Validates member access to prevent prototype pollution attacks. + * Blocks access to __proto__, prototype, and constructor properties. + * + * @param propertyName - The property name being accessed + * @param expressionString - The full expression string for error context + * @throws {AccessError} When trying to access dangerous prototype properties + */ + static validateMemberAccess(propertyName: string, expressionString: string): void { + if (DANGEROUS_PROPERTIES.has(propertyName)) { + throw new AccessError( + `Prototype access detected in member expression`, + { + propertyName, + expression: expressionString + } + ); + } + } + + /** + * Checks if a function is allowed to be called. + * Only functions explicitly registered in expr.functions or safe Math functions are allowed. + * + * @param fn - The function to check + * @param registeredFunctions - The registered functions from the expression's parser + * @returns true if the function is allowed, false otherwise + */ + static isAllowedFunction(fn: unknown, registeredFunctions: Record): boolean { + if (typeof fn !== 'function') { + return true; // Non-functions are not subject to function call restrictions + } + + // Check if it's a safe Math function + if (SAFE_MATH_FUNCTIONS.has(fn as Function)) { + return true; + } + + // Check if it's registered in expr.functions + for (const key in registeredFunctions) { + if (Object.prototype.hasOwnProperty.call(registeredFunctions, key) && registeredFunctions[key] === fn) { + return true; + } + } + + return false; + } + + /** + * Validates that a function is allowed to be called. + * Throws an error if the function is not in the allowed list. + * + * @param fn - The function to validate + * @param registeredFunctions - The registered functions from the expression's parser + * @param expressionString - The full expression string for error context + * @throws {FunctionError} When trying to call an unregistered function + */ + static validateAllowedFunction( + fn: unknown, + registeredFunctions: Record, + expressionString: string + ): void { + if (typeof fn === 'function' && !this.isAllowedFunction(fn, registeredFunctions)) { + throw new FunctionError( + 'Calling unregistered functions is not allowed for security reasons', + { + expression: expressionString + } + ); + } + } + /** * Validates function call parameters */ diff --git a/test/operators/operators-logical.ts b/test/operators/operators-logical.ts index d1b5dbdd..66322182 100644 --- a/test/operators/operators-logical.ts +++ b/test/operators/operators-logical.ts @@ -42,15 +42,21 @@ describe('Logical Operators TypeScript Test', () => { it('skips rhs when lhs is false', () => { const notCalled = spy(returnFalse); + // Security: Functions must be registered in parser.functions before use + const parser = new Parser(); + parser.functions.notCalled = notCalled; - expect(Parser.evaluate('false and notCalled()', { notCalled: notCalled })).toBe(false); + expect(parser.evaluate('false and notCalled()')).toBe(false); expect(notCalled.called).toBe(false); }); it('evaluates rhs when lhs is true', () => { const called = spy(returnFalse); + // Security: Functions must be registered in parser.functions before use + const parser = new Parser(); + parser.functions.called = called; - expect(Parser.evaluate('true and called()', { called: called })).toBe(false); + expect(parser.evaluate('true and called()')).toBe(false); expect(called.called).toBe(true); }); }); @@ -82,15 +88,21 @@ describe('Logical Operators TypeScript Test', () => { it('skips rhs when lhs is true', () => { const notCalled = spy(returnFalse); + // Security: Functions must be registered in parser.functions before use + const parser = new Parser(); + parser.functions.notCalled = notCalled; - expect(Parser.evaluate('true or notCalled()', { notCalled: notCalled })).toBe(true); + expect(parser.evaluate('true or notCalled()')).toBe(true); expect(notCalled.called).toBe(false); }); it('evaluates rhs when lhs is false', () => { const called = spy(returnTrue); + // Security: Functions must be registered in parser.functions before use + const parser = new Parser(); + parser.functions.called = called; - expect(Parser.evaluate('false or called()', { called: called })).toBe(true); + expect(parser.evaluate('false or called()')).toBe(true); expect(called.called).toBe(true); }); }); diff --git a/test/security/security.ts b/test/security/security.ts new file mode 100644 index 00000000..4a664ba6 --- /dev/null +++ b/test/security/security.ts @@ -0,0 +1,249 @@ +/** + * Security tests for CVE-2025-12735, CVE-2025-13204, and related vulnerabilities + * + * These tests verify that the library is protected against: + * 1. CVE-2025-12735: Code injection via arbitrary function calls in evaluation context + * 2. CVE-2025-13204: Prototype pollution via __proto__, prototype, constructor access + * 3. Bypass vulnerabilities via member function calls (Issue #289) + */ + +import { describe, it, expect } from 'vitest'; +import { Parser } from '../../index'; +import { AccessError, FunctionError } from '../../src/types/errors'; + +describe('Security Tests', () => { + describe('CVE-2025-12735: Code Injection Prevention', () => { + it('should block direct function calls passed via context', () => { + const parser = new Parser(); + const dangerousContext = { + dangerousFunc: () => 'pwned' + }; + + expect(() => parser.evaluate('dangerousFunc()', dangerousContext)) + .toThrow(FunctionError); + }); + + it('should block variable access to functions passed via context', () => { + const parser = new Parser(); + const dangerousContext = { + exec: () => 'pwned' + }; + + expect(() => parser.evaluate('exec("whoami")', dangerousContext)) + .toThrow(FunctionError); + }); + + it('should allow registered functions in parser.functions', () => { + const parser = new Parser(); + parser.functions.safeFunc = (x: number) => x * 2; + + expect(parser.evaluate('safeFunc(5)')).toBe(10); + }); + + it('should allow safe Math functions', () => { + const parser = new Parser(); + + expect(parser.evaluate('sin(0)')).toBe(0); + expect(parser.evaluate('cos(0)')).toBe(1); + expect(parser.evaluate('abs(-5)')).toBe(5); + expect(parser.evaluate('sqrt(4)')).toBe(2); + expect(parser.evaluate('pow(2, 3)')).toBe(8); + }); + + it('should allow inline-defined functions (IFUNDEF)', () => { + const parser = new Parser(); + + expect(parser.evaluate('(f(x) = x * 2)(5)')).toBe(10); + expect(parser.evaluate('f(x) = x * x; f(4)')).toBe(16); + }); + + it('should allow recursive inline functions', () => { + const parser = new Parser(); + + // Factorial function + expect(parser.evaluate('(f(x) = x > 1 ? x * f(x - 1) : 1)(5)')).toBe(120); + }); + }); + + describe('CVE-2025-13204: Prototype Pollution Prevention', () => { + it('should block __proto__ access in member expressions', () => { + const parser = new Parser(); + + expect(() => parser.evaluate('x.__proto__', { x: {} })) + .toThrow(AccessError); + }); + + it('should block prototype access in member expressions', () => { + const parser = new Parser(); + + expect(() => parser.evaluate('x.prototype', { x: function(): number { return 0; } })) + .toThrow(); // Can throw AccessError or FunctionError depending on the function check + }); + + it('should block __proto__ access in variable names', () => { + const parser = new Parser(); + + expect(() => parser.evaluate('__proto__', { '__proto__': {} })) + .toThrow(AccessError); + }); + + it('should block prototype access in variable names', () => { + const parser = new Parser(); + + expect(() => parser.evaluate('prototype', { prototype: {} })) + .toThrow(AccessError); + }); + + it('should block constructor access in variable names', () => { + const parser = new Parser(); + + expect(() => parser.evaluate('constructor', { constructor: {} })) + .toThrow(); + }); + }); + + describe('Issue #289: Member Function Call Bypass Prevention', () => { + it('should block member function calls on nested objects', () => { + const parser = new Parser(); + const dangerousContext = { + obj: { + dangerousMethod: () => 'pwned via member' + } + }; + + expect(() => parser.evaluate('obj.dangerousMethod()', dangerousContext)) + .toThrow(FunctionError); + }); + + it('should block deeply nested dangerous function calls', () => { + const parser = new Parser(); + const dangerousContext = { + level1: { + level2: { + exec: () => 'pwned deeply' + } + } + }; + + expect(() => parser.evaluate('level1.level2.exec()', dangerousContext)) + .toThrow(FunctionError); + }); + + it('should allow safe member access on objects', () => { + const parser = new Parser(); + const safeContext = { + user: { + name: 'John', + age: 30 + } + }; + + expect(parser.evaluate('user.name', safeContext)).toBe('John'); + expect(parser.evaluate('user.age', safeContext)).toBe(30); + }); + + it('should allow safe nested member access', () => { + const parser = new Parser(); + const safeContext = { + data: { + info: { + value: 42 + } + } + }; + + expect(parser.evaluate('data.info.value', safeContext)).toBe(42); + }); + }); + + describe('PoC Attacks from Security Reports', () => { + it('PoC: VU#263614 - deny child exec process', () => { + const parser = new Parser(); + const context = { + exec: () => 'executed' + }; + + expect(() => parser.evaluate('exec("whoami")', context)) + .toThrow(FunctionError); + }); + + it('PoC: Issue #289 by @baoquanh - nested dangerous function', () => { + const parser = new Parser(); + const baoquanh = { + test: { + write: () => 'file written' + } + }; + + expect(() => parser.evaluate('test.write("pwned.txt", "Hello!")', baoquanh)) + .toThrow(FunctionError); + }); + + it('PoC: write file via context - should be blocked', () => { + const parser = new Parser(); + const context = { + write: () => 'wrote file' + }; + + expect(() => parser.evaluate('write("pwned.txt", "Hello!")', context)) + .toThrow(FunctionError); + }); + }); + + describe('Safe Operations', () => { + it('should still allow normal arithmetic expressions', () => { + const parser = new Parser(); + + expect(parser.evaluate('2 + 3')).toBe(5); + expect(parser.evaluate('10 - 5')).toBe(5); + expect(parser.evaluate('4 * 5')).toBe(20); + expect(parser.evaluate('20 / 4')).toBe(5); + }); + + it('should still allow variables with primitive values', () => { + const parser = new Parser(); + + expect(parser.evaluate('x + y', { x: 10, y: 20 })).toBe(30); + expect(parser.evaluate('name', { name: 'test' })).toBe('test'); + expect(parser.evaluate('flag', { flag: true })).toBe(true); + }); + + it('should still allow array access', () => { + const parser = new Parser(); + + expect(parser.evaluate('arr[0]', { arr: [1, 2, 3] })).toBe(1); + expect(parser.evaluate('arr[1] + arr[2]', { arr: [1, 2, 3] })).toBe(5); + }); + + it('should still allow built-in functions', () => { + const parser = new Parser(); + + expect(parser.evaluate('min(5, 3)')).toBe(3); + expect(parser.evaluate('max(5, 3)')).toBe(5); + expect(parser.evaluate('floor(3.7)')).toBe(3); + expect(parser.evaluate('ceil(3.2)')).toBe(4); + }); + + it('should still allow string operations', () => { + const parser = new Parser(); + + expect(parser.evaluate('"hello" + " " + "world"')).toBe('hello world'); + expect(parser.evaluate('length "test"')).toBe(4); + }); + + it('should still allow conditional expressions', () => { + const parser = new Parser(); + + expect(parser.evaluate('x > 5 ? "big" : "small"', { x: 10 })).toBe('big'); + expect(parser.evaluate('x > 5 ? "big" : "small"', { x: 3 })).toBe('small'); + }); + + it('should still allow logical operators', () => { + const parser = new Parser(); + + expect(parser.evaluate('true and false')).toBe(false); + expect(parser.evaluate('true or false')).toBe(true); + expect(parser.evaluate('not true')).toBe(false); + }); + }); +});