Security fix for CVE-2025-12735, CVE-2025-13204, and member function bypass (#6)

* Initial plan

* Add security protections for CVE-2025-12735, CVE-2025-13204, and Issue silentmatt#289

- Block arbitrary function calls passed via evaluation context
- Block prototype pollution via __proto__, prototype, constructor access
- Block member function call bypass vulnerability
- Allow inline-defined functions (IFUNDEF) as they are safe
- Allow registered functions in parser.functions
- Allow safe Math functions (sin, cos, sqrt, etc.)
- Update tests to use secure function registration pattern
- Add comprehensive security test suite

Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>

* Fix code review issues: add validateAllowedFunction to IFUNCALL, use counter-based keys for inline functions

- Add validateAllowedFunction check in IFUNCALL handler to prevent bypassing security
- Use monotonically increasing counter for inline function keys instead of user-controlled names
- Prevent collision attacks on inline function registration

Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>

* Update tests to use secure function registration pattern and add BREAKING_CHANGES.md

- Update remaining 2 tests in operators-logical.ts to register functions in parser.functions
- Add comprehensive BREAKING_CHANGES.md documenting security changes and migration guide

Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>

* Fix TypeScript type error in security test

Change empty function to return number to satisfy Value type constraint

Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>

* Bump version to 5.0.0 for breaking changes per semantic versioning

- Update package.json version from 4.2.0 to 5.0.0
- Update BREAKING_CHANGES.md to reference version 5.0.0
- Update package-lock.json accordingly

Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>

Author: Copilot

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__: {} });
+```

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@pro-fa/expr-eval",
-  "version": "4.2.0",
+  "version": "5.0.0",
   "description": "Mathematical expression evaluator",
   "keywords": [
     "expression",

package.json

@@ -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: <something> }, use <something>
           // 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) {

src/core/evaluate.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<Function> = 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<string, OperatorFunction>): 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<string, OperatorFunction>,
+    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
    */

src/validation/expression-validator.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);
     });
   });