diff --git a/README.md b/README.md index 1472abb..41937d3 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,23 @@ JavaScript Expression Evaluator [![CDNJS version](https://img.shields.io/cdnjs/v/expr-eval.svg?maxAge=3600)](https://cdnjs.com/libraries/expr-eval) [![Build Status](https://travis-ci.org/silentmatt/expr-eval.svg?branch=master)](https://travis-ci.org/silentmatt/expr-eval) +This fork addresses https://github.com/silentmatt/expr-eval/issues/266, security fix has been committed but was never released to NPM +Therefore, we publish expr-eval-fork to NPM to work around this issue. +If expr-eval ever gets released, raise an issue and we'll deprecate this fork. + +This fork addresses a security vulnerability identified by CVE-2025-12735. +An important update with a strict allow-list security model to prevent +Code Injection (CWE-94) and Prototype Pollution (CWE-1321) vulnerabilities. +To achieve this, the expression evaluator no longer allows arbitrary +functions to be passed directly into the evaluation context +`(.evaluate({ myFunc: function() { ... } }))`. Any external function that +is intended for use in an expression must be explicitly registered with +the Parser instance via the parser.functions map prior to evaluation. This +impacts very few test cases that have been updated in test/*.js. A new +`test/security.js` highlights the attacks against vulnerbaility +CVE-2025-12735 that will be prevented. + + Description ------------------------------------- diff --git a/package.json b/package.json index 36260e0..847cca0 100644 --- a/package.json +++ b/package.json @@ -1,26 +1,25 @@ { "name": "expr-eval", - "version": "2.0.2", - "description": "Mathematical expression evaluator", + "version": "3.0.1", + "description": "Mathematical expression evaluator with security", "main": "dist/bundle.js", "module": "dist/index.mjs", "typings": "parser.d.ts", "directories": { "test": "test" }, - "dependencies": {}, "devDependencies": { - "eslint": "^6.3.0", - "eslint-config-semistandard": "^15.0.0", - "eslint-config-standard": "^13.0.1", - "eslint-plugin-import": "^2.15.0", - "eslint-plugin-node": "^9.2.0", - "eslint-plugin-promise": "^4.0.1", - "eslint-plugin-standard": "^4.0.0", - "mocha": "^6.2.0", - "nyc": "^14.1.1", - "rollup": "^1.20.3", - "rollup-plugin-uglify": "^6.0.3" + "eslint": "^8.57.0", + "eslint-config-semistandard": "^17.0.0", + "eslint-config-standard": "^17.1.0", + "eslint-plugin-import": "^2.32.0", + "eslint-plugin-node": "^11.1.0", + "eslint-plugin-promise": "^6.0.0", + "eslint-plugin-standard": "^5.0.0", + "mocha": "^11.7.4", + "nyc": "^17.1.0", + "rollup": "^4.52.5", + "@rollup/plugin-terser": "^0.4.4" }, "scripts": { "test": "npm run build && mocha", diff --git a/rollup-esm.config.js b/rollup-esm.config.js index d530a43..d4b84db 100644 --- a/rollup-esm.config.js +++ b/rollup-esm.config.js @@ -1,7 +1,19 @@ -import rollupConfig from './rollup.config'; +// rollup-esm.config.js (Converted to CommonJS) -rollupConfig.plugins = []; -rollupConfig.output.file = 'dist/index.mjs'; -rollupConfig.output.format = 'esm'; +// 1. Use require() to import the base config +const rollupConfig = require('./rollup.config.js'); -export default rollupConfig; +// Create a copy of the base configuration object to avoid modifying it +// This is critical since 'rollupConfig' is a shared object. +const esmConfig = { ...rollupConfig }; + +// 2. Apply ESM-specific changes to the copied object +esmConfig.plugins = []; // No minification for the base ESM file +esmConfig.output = { + ...rollupConfig.output, + file: 'dist/index.mjs', + format: 'esm' +}; + +// 3. Use module.exports to export the configuration +module.exports = esmConfig; diff --git a/rollup-min.config.js b/rollup-min.config.js index 222fafa..fb39760 100644 --- a/rollup-min.config.js +++ b/rollup-min.config.js @@ -1,7 +1,21 @@ -import rollupConfig from './rollup.config'; -import { uglify } from 'rollup-plugin-uglify'; +// rollup-min.config.js (Correct CommonJS Syntax) -rollupConfig.plugins = [ uglify() ]; -rollupConfig.output.file = 'dist/bundle.min.js'; +// 1. Use require() to import the base config and the terser plugin +const rollupConfig = require('./rollup.config.js'); +const terser = require('@rollup/plugin-terser'); -export default rollupConfig; +// Create a shallow copy of the base configuration to avoid side effects +const minifiedConfig = { + ...rollupConfig, + // Ensure the output property is also copied, if it exists + output: { ...rollupConfig.output } +}; + +// 2. Set the plugins to ONLY include the terser plugin +minifiedConfig.plugins = [ terser() ]; + +// 3. Update the file name +minifiedConfig.output.file = 'dist/bundle.min.js'; + +// 4. Use module.exports to export the configuration +module.exports = minifiedConfig; diff --git a/rollup.config.js b/rollup.config.js index 1c55fc7..6d2d7b6 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -1,4 +1,5 @@ -export default { +// rollup.config.js (CommonJS syntax) +module.exports = { input: 'index.js', output: { file: 'dist/bundle.js', diff --git a/src/evaluate.js b/src/evaluate.js index f23cef7..f9aa992 100644 --- a/src/evaluate.js +++ b/src/evaluate.js @@ -9,6 +9,30 @@ export default function evaluate(tokens, expr, values) { return resolveExpression(tokens, values); } + /** + * Checks if a function reference 'f' is explicitly allowed to be executed. + * This logic is the core security allowance gate. + */ + var isAllowedFunc = function (f) { + if (typeof f !== 'function') return true; + + for (var key in expr.functions) { + if (expr.functions[key] === f) return true; + } + + if (f.__expr_eval_safe_def) return true; + + for (var key in values) { + if (typeof values[key] === 'object' && values[key] !== null) { + for (var subKey in values[key]) { + if (values[key][subKey] === f) return true; + } + } + } + return false; + }; + /* --- END: LOCAL HELPER FUNCTION FOR SECURITY --- */ + var numTokens = tokens.length; for (var i = 0; i < numTokens; i++) { @@ -50,7 +74,11 @@ export default function evaluate(tokens, expr, values) { nstack.push(expr.unaryOps[item.value]); } else { var v = values[item.value]; - if (v !== undefined) { + if (v !== undefined) { + if (typeof v === 'function' && !isAllowedFunc(v)) { + /* function is not registered, not marked safe, and not a member function. BLOCKED. */ + throw new Error('Variable references an unallowed function: ' + item.value); + } nstack.push(v); } else { throw new Error('undefined variable: ' + item.value); @@ -66,7 +94,14 @@ export default function evaluate(tokens, expr, values) { while (argCount-- > 0) { args.unshift(resolveExpression(nstack.pop(), values)); } - f = nstack.pop(); + f = nstack.pop(); + + // --- FINAL SECURITY CHECK --- + if (!isAllowedFunc(f)) { + throw new Error('Is not an allowed function.'); + } + // --- END FINAL SECURITY CHECK --- + if (f.apply && f.call) { nstack.push(f.apply(undefined, args)); } else { @@ -94,6 +129,12 @@ export default function evaluate(tokens, expr, values) { value: n1, writable: false }); + // *** MARK AS SAFE FOR SECURITY CHECK *** + Object.defineProperty(f, '__expr_eval_safe_def', { + value: true, + writable: false + }); + // *************************************** values[n1] = f; return f; })()); diff --git a/test/operators.js b/test/operators.js index 07d15aa..d665698 100644 --- a/test/operators.js +++ b/test/operators.js @@ -162,17 +162,17 @@ describe('Operators', function () { assert.strictEqual(Parser.evaluate('1 and 1 and 0'), false); }); - it('skips rhs when lhs is false', function () { + it('skips rhs when lhs is false for AND operator', function () { var notCalled = spy(returnFalse); assert.strictEqual(Parser.evaluate('false and notCalled()', { notCalled: notCalled }), false); assert.strictEqual(notCalled.called, false); }); - it('evaluates rhs when lhs is true', function () { + it('evaluates rhs when lhs is true for AND operator', function () { var called = spy(returnFalse); - assert.strictEqual(Parser.evaluate('true and called()', { called: called }), false); + assert.strictEqual(Parser.evaluate('true and spies.called()', { spies: {called: called }}), false); assert.strictEqual(called.called, true); }); }); @@ -212,7 +212,7 @@ describe('Operators', function () { it('evaluates rhs when lhs is false', function () { var called = spy(returnTrue); - assert.strictEqual(Parser.evaluate('false or called()', { called: called }), true); + assert.strictEqual(Parser.evaluate('false or spies.called()', { spies: {called: called }}), true); assert.strictEqual(called.called, true); }); }); diff --git a/test/parser.js b/test/parser.js index bf4459d..bd3831c 100644 --- a/test/parser.js +++ b/test/parser.js @@ -234,7 +234,8 @@ describe('Parser', function () { assert.strictEqual(parser.parse('sin;').toString(), '(sin)'); assert.strictEqual(parser.parse('(sin)').toString(), 'sin'); assert.strictEqual(parser.parse('sin; (2)^3').toString(), '(sin;(2 ^ 3))'); - assert.deepStrictEqual(parser.parse('f(sin, sqrt)').evaluate({ f: function (a, b) { return [ a, b ]; }}), [ Math.sin, Math.sqrt ]); + /* After this update, to pass a test function f to be used within an expression, you must now register it. The old, insecure pattern of passing the function directly in the evaluate context: parser.parse('f(sin)').evaluate({ f: myFunc }) will now throw an exception. Instead, you must pre-register the function, which is often best done using a concise Immediate Invoked Function Expression (IIFE) for test cases: (function() { parser.functions.f = myFunc; return parser.parse('f(sin)').evaluate();})(). This explicit registration guarantees the function is trusted and safe for execution. */ + assert.deepStrictEqual((function() { parser.functions.f = function (a, b) { return [ a, b ]; }; return parser.parse('f(sin, sqrt)').evaluate();})(), [ Math.sin, Math.sqrt ]); assert.strictEqual(parser.parse('sin').evaluate(), Math.sin); assert.strictEqual(parser.parse('cos;').evaluate(), Math.cos); assert.strictEqual(parser.parse('cos;tan').evaluate(), Math.tan); diff --git a/test/security.js b/test/security.js new file mode 100644 index 0000000..d688d48 --- /dev/null +++ b/test/security.js @@ -0,0 +1,49 @@ +'use strict'; +var assert = require('assert'); +var Parser = require('../dist/bundle').Parser; +var fs = require('fs'); +var child_process = require('child_process'); + +/* A context of potential dangerous stuff */ +var context = { + write: (path, data) => fs.writeFileSync(path, data), + cmd: (cmd) => console.log('Executing:', cmd), + exec: child_process.execSync +}; + +it('should fail on direct function call to an unallowed function', function () { + var parser = new Parser(); + assert.throws(() => { + parser.evaluate('write("pwned.txt","Hello!")', context); + }, Error); +}); + +it('should allow IFUNDEF but keep function calls safe', function () { + var parserWithFndef = new Parser({ + operators: { fndef: true } + }); + var safeExpr = '(f(x) = x * x)(5)'; + assert.equal(parserWithFndef.evaluate(safeExpr), 25, + 'Should correctly evaluate an expression with an allowed IFUNDEF.'); + var dangerousExpr = '((h(x) = write("pwned.txt", x)) + h(5))'; + assert.throws(() => { + parserWithFndef.evaluate(dangerousExpr, context); + }, Error); +}); + +it('should fail when a variable is assigned a dangerous function', function () { + var parser = new Parser(); + + var dangerousContext = { ...context, evil: context.cmd }; + + assert.throws(() => { + parser.evaluate('evil("ls -lh /")', dangerousContext); + }, Error); +}); + +it('PoC provided by researcher VU#263614 deny child exec process', function() { + var parser = new Parser(); + assert.throws(() => { + parser.evaluate('exec("whoami")', context); + }, Error); +});