From f79a00ff8360452f93b693249496cb835edb7c05 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 11:51:42 +0200 Subject: [PATCH 01/10] fix: disable parser functions in the CLI (security issue) --- bin/cli.js | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 4943276d29..dc1da38eba 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -56,10 +56,28 @@ const PRECISION = 14 // decimals * "Lazy" load math.js: only require when we actually start using it. * This ensures the cli application looks like it loads instantly. * When requesting help or version number, math.js isn't even loaded. - * @return {*} + * @return {{ evalute: function, parse: function, math: Object }} */ function getMath () { - return require('../lib/browser/math.js') + const { create, all } = require('../lib/browser/math.js') + + const math = create(all) + const parse = math.parse + const evaluate = math.evaluate + + // See https://mathjs.org/docs/expressions/security.html#less-vulnerable-expression-parser + math.import({ + 'import': function () { throw new Error('Function import is disabled') }, + 'createUnit': function () { throw new Error('Function createUnit is disabled') }, + 'evaluate': function () { throw new Error('Function evaluate is disabled') }, + 'parse': function () { throw new Error('Function parse is disabled') }, + 'simplify': function () { throw new Error('Function simplify is disabled') }, + 'derivative': function () { throw new Error('Function derivative is disabled') }, + 'resolve': function () { throw new Error('Function resolve is disabled') }, + 'reviver': function () { throw new Error('Function reviver is disabled') } + }, { override: true }) + + return { math, parse, evaluate } } /** @@ -68,7 +86,7 @@ function getMath () { * @param {*} value */ function format (value) { - const math = getMath() + const { math } = getMath() return math.format(value, { fn: function (value) { @@ -88,7 +106,7 @@ function format (value) { * @return {[Array, String]} completions */ function completer (text) { - const math = getMath() + const { math } = getMath() let matches = [] let keyword const m = /[a-zA-Z_0-9]+$/.exec(text) @@ -183,7 +201,7 @@ function runStream (input, output, mode, parenthesis) { } // load math.js now, right *after* loading the prompt. - const math = getMath() + const { math, parse } = getMath() // TODO: automatic insertion of 'ans' before operators like +, -, *, / @@ -214,7 +232,7 @@ function runStream (input, output, mode, parenthesis) { case 'evaluate': // evaluate expression try { - let node = math.parse(expr) + let node = parse(expr) let res = node.evaluate(scope) if (math.isResultSet(res)) { @@ -288,7 +306,7 @@ function runStream (input, output, mode, parenthesis) { * @return {string | null} Returns the name when found, else returns null. */ function findSymbolName (node) { - const math = getMath() + const { math } = getMath() let n = node while (n) { @@ -412,9 +430,10 @@ if (version) { // run a stream, can be user input or pipe input runStream(process.stdin, process.stdout, mode, parenthesis) } else { - fs.stat(scripts[0], function (e, f) { - if (e) { - console.log(getMath().evaluate(scripts.join(' ')).toString()) + fs.stat(scripts[0], function (err) { + if (err) { + const { evaluate } = getMath() + console.log(evaluate(scripts.join(' ')).toString()) } else { // work through the queue of scripts scripts.forEach(function (arg) { From f5107bf566504bb29574d15b6c9da4b11ffb6f6b Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 11:53:04 +0200 Subject: [PATCH 02/10] fix: ensure `ObjectWrappingMap` doesn't allow deleting unsafe properties (security issue) --- src/utils/map.js | 8 +- test/unit-tests/utils/map.test.js | 186 +++++++++++++++++------------- 2 files changed, 113 insertions(+), 81 deletions(-) diff --git a/src/utils/map.js b/src/utils/map.js index 30cd467754..221b98bbd0 100644 --- a/src/utils/map.js +++ b/src/utils/map.js @@ -1,4 +1,4 @@ -import { getSafeProperty, hasSafeProperty, setSafeProperty } from './customs.js' +import { getSafeProperty, isSafeProperty, setSafeProperty } from './customs.js' import { isMap, isObject } from './is.js' /** @@ -30,7 +30,7 @@ export class ObjectWrappingMap { } has (key) { - return hasSafeProperty(this.wrappedObject, key) + return isSafeProperty(this.wrappedObject, key) && key in this.wrappedObject } entries () { @@ -44,7 +44,9 @@ export class ObjectWrappingMap { } delete (key) { - delete this.wrappedObject[key] + if (isSafeProperty(this.wrappedObject, key)) { + delete this.wrappedObject[key] + } } clear () { diff --git a/test/unit-tests/utils/map.test.js b/test/unit-tests/utils/map.test.js index 19f9fbc021..2ef92b4cfe 100644 --- a/test/unit-tests/utils/map.test.js +++ b/test/unit-tests/utils/map.test.js @@ -30,95 +30,125 @@ describe('maps', function () { } }) - it('should wrap an object in a map-like object', function () { - const obj = { - a: 1, - b: 2, - c: 3 - } - const map = new ObjectWrappingMap(obj) + describe('ObjectWrappingMap', function () { + it('should wrap an object in a map-like object', function () { + const obj = { + a: 1, + b: 2, + c: 3 + } + const map = new ObjectWrappingMap(obj) + + // isMap thinks it's a map. + assert.ok(isMap(map)) + + // get + for (const key of ['a', 'b', 'c']) { + assert.strictEqual(map.get(key), obj[key]) + } + + // get with a key not there gives an undefined. + for (const key of ['e', 'f']) { + assert.strictEqual(map.get(key), undefined) + } + + // We can behind the scenes add to the wrapped object. + Object.assign(obj, { + d: 4, + e: 5 + }) + + // set() + map.set('f', 6) + map.set('g', 7) + + // we set the properties in obj, too. + assert.strictEqual(obj.f, 6) + assert.strictEqual(obj.g, 7) + + for (const key of ['d', 'e', 'f', 'g']) { + assert.strictEqual(map.get(key), obj[key]) + } + + // keys() + assert.deepStrictEqual([...map.keys()], ['a', 'b', 'c', 'd', 'e', 'f', 'g']) + + for (const key of map.keys()) { + assert.ok(map.has(key)) + } + + // size( + assert.strictEqual(map.size, 7) + + // delete + map.delete('g') + assert.deepStrictEqual([...map.keys()], ['a', 'b', 'c', 'd', 'e', 'f']) + + assert.ok(!map.has('not-in-this-map')) + + // forEach + const log = [] + map.forEach((value, key) => (log.push([key, value]))) + assert.deepStrictEqual(log, [ + ['a', 1], + ['b', 2], + ['c', 3], + ['d', 4], + ['e', 5], + ['f', 6] + ]) - // isMap thinks it's a map. - assert.ok(isMap(map)) + // entries + const it = map.entries() + assert.deepStrictEqual(it.next(), { done: false, value: ['a', 1] }) + assert.deepStrictEqual(it.next(), { done: false, value: ['b', 2] }) + assert.deepStrictEqual(it.next(), { done: false, value: ['c', 3] }) + assert.deepStrictEqual(it.next(), { done: false, value: ['d', 4] }) + assert.deepStrictEqual(it.next(), { done: false, value: ['e', 5] }) + assert.deepStrictEqual(it.next(), { done: false, value: ['f', 6] }) + assert.deepStrictEqual(it.next(), { done: true, value: undefined }) - // get - for (const key of ['a', 'b', 'c']) { - assert.strictEqual(map.get(key), obj[key]) - } + // We can get the same object out using toObject + const innerObject = toObject(map) + assert.strictEqual(innerObject, obj) - // get with a key not there gives an undefined. - for (const key of ['e', 'f']) { - assert.strictEqual(map.get(key), undefined) - } + // Create a new Map + const copy = new Map(map) + assert.deepStrictEqual([...copy.keys()], [...map.keys()]) - // We can behind the scenes add to the wrapped object. - Object.assign(obj, { - d: 4, - e: 5 + // clear + map.clear() + assert.deepStrictEqual([...map.keys()], []) + assert.deepStrictEqual(Object.keys(obj), []) }) - // set() - map.set('f', 6) - map.set('g', 7) + it('should not allow getting unsafe properties' , function () { + const map = new ObjectWrappingMap({}) - // we set the properties in obj, too. - assert.strictEqual(obj.f, 6) - assert.strictEqual(obj.g, 7) + assert.throws(() => map.get('__proto__'), /Error: No access to property "__proto__"/) + }) - for (const key of ['d', 'e', 'f', 'g']) { - assert.strictEqual(map.get(key), obj[key]) - } + it('should not allow setting unsafe properties' , function () { + const map = new ObjectWrappingMap({}) - // keys() - assert.deepStrictEqual([...map.keys()], ['a', 'b', 'c', 'd', 'e', 'f', 'g']) + assert.throws(() => map.set('__proto__', 42), /Error: No access to property "__proto__"/) + }) - for (const key of map.keys()) { - assert.ok(map.has(key)) - } + it('should not allow testing has unsafe properties' , function () { + const map = new ObjectWrappingMap({}) - // size( - assert.strictEqual(map.size, 7) - - // delete - map.delete('g') - assert.deepStrictEqual([...map.keys()], ['a', 'b', 'c', 'd', 'e', 'f']) - - assert.ok(!map.has('not-in-this-map')) - - // forEach - const log = [] - map.forEach((value, key) => (log.push([key, value]))) - assert.deepStrictEqual(log, [ - ['a', 1], - ['b', 2], - ['c', 3], - ['d', 4], - ['e', 5], - ['f', 6] - ]) - - // entries - const it = map.entries() - assert.deepStrictEqual(it.next(), { done: false, value: ['a', 1] }) - assert.deepStrictEqual(it.next(), { done: false, value: ['b', 2] }) - assert.deepStrictEqual(it.next(), { done: false, value: ['c', 3] }) - assert.deepStrictEqual(it.next(), { done: false, value: ['d', 4] }) - assert.deepStrictEqual(it.next(), { done: false, value: ['e', 5] }) - assert.deepStrictEqual(it.next(), { done: false, value: ['f', 6] }) - assert.deepStrictEqual(it.next(), { done: true, value: undefined }) - - // We can get the same object out using toObject - const innerObject = toObject(map) - assert.strictEqual(innerObject, obj) - - // Create a new Map - const copy = new Map(map) - assert.deepStrictEqual([...copy.keys()], [...map.keys()]) - - // clear - map.clear() - assert.deepStrictEqual([...map.keys()], []) - assert.deepStrictEqual(Object.keys(obj), []) + assert.strictEqual(map.has('__proto__'), false) + }) + + it('should not allow deleting unsafe properties' , function () { + const obj = { + toString: 42 + } + const map = new ObjectWrappingMap(obj) + + map.delete('toString') + assert.strictEqual(obj.toString, 42) + }) }) describe('PartitionedMap', function () { From bda5f3ddabd69056b0b9c613348c1e71a21c459a Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 11:54:15 +0200 Subject: [PATCH 03/10] fix: enable using methods and (safe) properties on plain arrays --- src/utils/customs.js | 19 +++++-------------- test/unit-tests/utils/customs.test.js | 16 +++++++++------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/utils/customs.js b/src/utils/customs.js index d17b1e63af..beec3181aa 100644 --- a/src/utils/customs.js +++ b/src/utils/customs.js @@ -10,7 +10,7 @@ import { hasOwnProperty } from './object.js' */ function getSafeProperty (object, prop) { // only allow getting safe properties of a plain object - if (isPlainObject(object) && isSafeProperty(object, prop)) { + if (isSafeProperty(object, prop)) { return object[prop] } @@ -33,7 +33,7 @@ function getSafeProperty (object, prop) { // TODO: merge this function into access.js? function setSafeProperty (object, prop, value) { // only allow setting safe properties of a plain object - if (isPlainObject(object) && isSafeProperty(object, prop)) { + if (isSafeProperty(object, prop)) { object[prop] = value return value } @@ -41,22 +41,15 @@ function setSafeProperty (object, prop, value) { throw new Error('No access to property "' + prop + '"') } -function getSafeProperties (object) { - return Object.keys(object).filter((prop) => hasOwnProperty(object, prop)) -} - -function hasSafeProperty (object, prop) { - return prop in object -} - /** - * Test whether a property is safe to use for an object. + * Test whether a property is safe to use on an object or Array. * For example .toString and .constructor are not safe + * @param {Object | Array} object * @param {string} prop * @return {boolean} Returns true when safe */ function isSafeProperty (object, prop) { - if (!object || typeof object !== 'object') { + if (!isPlainObject(object) && !Array.isArray(object)) { return false } // SAFE: whitelisted @@ -158,8 +151,6 @@ const safeNativeMethods = { export { getSafeProperty } export { setSafeProperty } export { isSafeProperty } -export { hasSafeProperty } -export { getSafeProperties } export { getSafeMethod } export { isSafeMethod } export { isPlainObject } diff --git a/test/unit-tests/utils/customs.test.js b/test/unit-tests/utils/customs.test.js index 943914235f..bb7f4cd0a8 100644 --- a/test/unit-tests/utils/customs.test.js +++ b/test/unit-tests/utils/customs.test.js @@ -150,14 +150,14 @@ describe('customs', function () { assert.strictEqual(isSafeProperty(object, 'arguments'), false) assert.strictEqual(isSafeProperty(object, 'caller'), false) - // non existing property + // non-existing property assert.strictEqual(isSafeProperty(object, 'bar'), true) // property with unicode chars assert.strictEqual(isSafeProperty(object, 'co\u006Estructor'), false) }) - it('should test inherited properties on plain objects ', function () { + it('should test inherited properties on plain objects', function () { const object1 = {} const object2 = Object.create(object1) object1.foo = true @@ -172,11 +172,13 @@ describe('customs', function () { assert.strictEqual(isSafeProperty(object2, 'constructor'), false) }) - it('should test for ghosted native property', function () { - const array1 = [] - const array2 = Object.create(array1) - array2.length = Infinity - assert.strictEqual(isSafeProperty(array2, 'length'), true) + it('should test properties on an array', function () { + const array = [3,2,1] + assert.strictEqual(isSafeProperty(array, 'length'), true) + assert.strictEqual(isSafeProperty(array, 'foo'), true) + assert.strictEqual(isSafeProperty(array, 'sort'), true) + assert.strictEqual(isSafeProperty(array, '__proto__'), false) + assert.strictEqual(isSafeProperty(array, 'constructor'), false) }) }) From fc5f51615b8f96af72c0420aada0d6547237be82 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 11:57:36 +0200 Subject: [PATCH 04/10] docs: update the "Less vulnerable expression parser" section in the docs --- docs/expressions/security.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/expressions/security.md b/docs/expressions/security.md index 03c90929dc..3b35efe3fe 100644 --- a/docs/expressions/security.md +++ b/docs/expressions/security.md @@ -40,8 +40,9 @@ risk in the expression parser: - `import` and `createUnit` which alter the built-in functionality and allow overriding existing functions and units. -- `evaluate`, `parse`, `simplify`, and `derivative` which parse arbitrary - input into a manipulable expression tree. +- `evaluate`, `parse`, `simplify`, `derivative`, and `resolve` which parse + arbitrary input into a manipulable expression tree. +- `reviver` which parses values into class instances. To make the expression parser less vulnerable whilst still supporting most functionality, these functions can be disabled: @@ -58,7 +59,9 @@ math.import({ 'evaluate': function () { throw new Error('Function evaluate is disabled') }, 'parse': function () { throw new Error('Function parse is disabled') }, 'simplify': function () { throw new Error('Function simplify is disabled') }, - 'derivative': function () { throw new Error('Function derivative is disabled') } + 'derivative': function () { throw new Error('Function derivative is disabled') }, + 'resolve': function () { throw new Error('Function derivative is disabled') }, + 'reviver': function () { throw new Error('Function derivative is disabled') } }, { override: true }) console.log(limitedEvaluate('sqrt(16)')) // Ok, 4 @@ -71,7 +74,7 @@ console.log(limitedEvaluate('parse("2+3")')) // Error: Function parse is disable You found a security vulnerability? Awesome! We hope you don't have bad intentions and want to help fix the issue. Please report the vulnerability in a private way by contacting one of the maintainers -via mail or an other private channel. That way we can work together +via mail or another private channel. That way we can work together on a fix before sharing the issue with everybody including the bad guys. ## Stability risks From 88d4654f40726a8687ada3eae11877d71bd0f49e Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 12:00:00 +0200 Subject: [PATCH 05/10] chore: fix typos and linting issues --- docs/expressions/security.md | 4 ++-- test/unit-tests/utils/customs.test.js | 2 +- test/unit-tests/utils/map.test.js | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/expressions/security.md b/docs/expressions/security.md index 3b35efe3fe..a1a963e079 100644 --- a/docs/expressions/security.md +++ b/docs/expressions/security.md @@ -60,8 +60,8 @@ math.import({ 'parse': function () { throw new Error('Function parse is disabled') }, 'simplify': function () { throw new Error('Function simplify is disabled') }, 'derivative': function () { throw new Error('Function derivative is disabled') }, - 'resolve': function () { throw new Error('Function derivative is disabled') }, - 'reviver': function () { throw new Error('Function derivative is disabled') } + 'resolve': function () { throw new Error('Function resolve is disabled') }, + 'reviver': function () { throw new Error('Function reviver is disabled') } }, { override: true }) console.log(limitedEvaluate('sqrt(16)')) // Ok, 4 diff --git a/test/unit-tests/utils/customs.test.js b/test/unit-tests/utils/customs.test.js index bb7f4cd0a8..3010cfc0a8 100644 --- a/test/unit-tests/utils/customs.test.js +++ b/test/unit-tests/utils/customs.test.js @@ -173,7 +173,7 @@ describe('customs', function () { }) it('should test properties on an array', function () { - const array = [3,2,1] + const array = [3, 2, 1] assert.strictEqual(isSafeProperty(array, 'length'), true) assert.strictEqual(isSafeProperty(array, 'foo'), true) assert.strictEqual(isSafeProperty(array, 'sort'), true) diff --git a/test/unit-tests/utils/map.test.js b/test/unit-tests/utils/map.test.js index 2ef92b4cfe..08805acc07 100644 --- a/test/unit-tests/utils/map.test.js +++ b/test/unit-tests/utils/map.test.js @@ -122,25 +122,25 @@ describe('maps', function () { assert.deepStrictEqual(Object.keys(obj), []) }) - it('should not allow getting unsafe properties' , function () { + it('should not allow getting unsafe properties', function () { const map = new ObjectWrappingMap({}) assert.throws(() => map.get('__proto__'), /Error: No access to property "__proto__"/) }) - it('should not allow setting unsafe properties' , function () { + it('should not allow setting unsafe properties', function () { const map = new ObjectWrappingMap({}) assert.throws(() => map.set('__proto__', 42), /Error: No access to property "__proto__"/) }) - it('should not allow testing has unsafe properties' , function () { + it('should not allow testing has unsafe properties', function () { const map = new ObjectWrappingMap({}) assert.strictEqual(map.has('__proto__'), false) }) - it('should not allow deleting unsafe properties' , function () { + it('should not allow deleting unsafe properties', function () { const obj = { toString: 42 } From 6aabc4b784208bd212aed14b708753599db02a4f Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 12:56:57 +0200 Subject: [PATCH 06/10] chore: keep functions like `simplify` enabled in the CLI --- bin/cli.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index dc1da38eba..6d31390894 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -69,12 +69,7 @@ function getMath () { math.import({ 'import': function () { throw new Error('Function import is disabled') }, 'createUnit': function () { throw new Error('Function createUnit is disabled') }, - 'evaluate': function () { throw new Error('Function evaluate is disabled') }, - 'parse': function () { throw new Error('Function parse is disabled') }, - 'simplify': function () { throw new Error('Function simplify is disabled') }, - 'derivative': function () { throw new Error('Function derivative is disabled') }, - 'resolve': function () { throw new Error('Function resolve is disabled') }, - 'reviver': function () { throw new Error('Function reviver is disabled') } + 'reviver': function () { throw new Error('Function reviver is disabled') } }, { override: true }) return { math, parse, evaluate } From 9abf6762ace5f1ee6360ba4e4aff8a7e70ebf6c9 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 13:42:47 +0200 Subject: [PATCH 07/10] docs: update the security page --- docs/expressions/security.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/expressions/security.md b/docs/expressions/security.md index a1a963e079..dbe9a9655b 100644 --- a/docs/expressions/security.md +++ b/docs/expressions/security.md @@ -39,10 +39,10 @@ There is a small number of functions which yield the biggest security risk in the expression parser: - `import` and `createUnit` which alter the built-in functionality and - allow overriding existing functions and units. + allow overriding existing functions and units, and `reviver` which parses + values into class instances. - `evaluate`, `parse`, `simplify`, `derivative`, and `resolve` which parse arbitrary input into a manipulable expression tree. -- `reviver` which parses values into class instances. To make the expression parser less vulnerable whilst still supporting most functionality, these functions can be disabled: @@ -54,14 +54,17 @@ const math = create(all) const limitedEvaluate = math.evaluate math.import({ + // most important (hardly any functionaly impact) 'import': function () { throw new Error('Function import is disabled') }, 'createUnit': function () { throw new Error('Function createUnit is disabled') }, + 'reviver': function () { throw new Error('Function reviver is disabled') }, + + // extra (has functional impact) 'evaluate': function () { throw new Error('Function evaluate is disabled') }, 'parse': function () { throw new Error('Function parse is disabled') }, 'simplify': function () { throw new Error('Function simplify is disabled') }, 'derivative': function () { throw new Error('Function derivative is disabled') }, 'resolve': function () { throw new Error('Function resolve is disabled') }, - 'reviver': function () { throw new Error('Function reviver is disabled') } }, { override: true }) console.log(limitedEvaluate('sqrt(16)')) // Ok, 4 From 5d8ee147f5b5b8a75374891c543d22e2108808cc Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 26 Aug 2024 19:10:14 +0200 Subject: [PATCH 08/10] fix: ensure `ObjectWrappingMap.keys` cannot list unsafe properties --- src/utils/map.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utils/map.js b/src/utils/map.js index 221b98bbd0..4affb59d6b 100644 --- a/src/utils/map.js +++ b/src/utils/map.js @@ -17,7 +17,9 @@ export class ObjectWrappingMap { } keys () { - return Object.keys(this.wrappedObject).values() + return Object.keys(this.wrappedObject) + .filter(key => this.has(key)) + .values() } get (key) { From d5a4cce7329f77a42ce0162a2a8fcffa13cbca1d Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Tue, 27 Aug 2024 16:22:58 +0200 Subject: [PATCH 09/10] fix: when overwriting a rawArgs function with a non-rawArgs function it was still called with raw arguments --- src/expression/node/FunctionNode.js | 13 ++++++++++--- .../unit-tests/expression/node/FunctionNode.test.js | 8 +++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/expression/node/FunctionNode.js b/src/expression/node/FunctionNode.js index 768ea6c03d..7de973c39d 100644 --- a/src/expression/node/FunctionNode.js +++ b/src/expression/node/FunctionNode.js @@ -169,7 +169,15 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({ const rawArgs = this.args return function evalFunctionNode (scope, args, context) { const fn = resolveFn(scope) - return fn(rawArgs, math, createSubScope(scope, args)) + + // the original function can be overwritten in the scope with a non-rawArgs function + if (fn.rawArgs === true) { + return fn(rawArgs, math, createSubScope(scope, args)) + } else { + // "regular" evaluation + const values = evalArgs.map((evalArg) => evalArg(scope, args, context)) + return fn(...values) + } } } else { // "regular" evaluation @@ -196,8 +204,7 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({ } default: return function evalFunctionNode (scope, args, context) { const fn = resolveFn(scope) - const values = evalArgs.map( - (evalArg) => evalArg(scope, args, context)) + const values = evalArgs.map((evalArg) => evalArg(scope, args, context)) return fn(...values) } } diff --git a/test/unit-tests/expression/node/FunctionNode.test.js b/test/unit-tests/expression/node/FunctionNode.test.js index cfc1ce1d6c..bc6c38426c 100644 --- a/test/unit-tests/expression/node/FunctionNode.test.js +++ b/test/unit-tests/expression/node/FunctionNode.test.js @@ -169,12 +169,18 @@ describe('FunctionNode', function () { const b = new mymath.ConstantNode(5) const n = new mymath.FunctionNode(s, [a, b]) + let actualArgs const scope = { - myFunction: function () { + myFunction: function (...args) { + actualArgs = args return 42 } } + assert.strictEqual(n.compile().evaluate(scope), 42) + assert.strictEqual(actualArgs[0], a.value) + assert.strictEqual(actualArgs[1], b.value) + assert.deepStrictEqual(actualArgs.length, 2) }) it('should filter a FunctionNode', function () { From 89720f36e41ab3b22012e5628ccc972d1ac7698c Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Tue, 27 Aug 2024 16:39:39 +0200 Subject: [PATCH 10/10] docs: fix a typo --- docs/expressions/security.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/expressions/security.md b/docs/expressions/security.md index dbe9a9655b..62ac1df8cf 100644 --- a/docs/expressions/security.md +++ b/docs/expressions/security.md @@ -54,7 +54,7 @@ const math = create(all) const limitedEvaluate = math.evaluate math.import({ - // most important (hardly any functionaly impact) + // most important (hardly any functional impact) 'import': function () { throw new Error('Function import is disabled') }, 'createUnit': function () { throw new Error('Function createUnit is disabled') }, 'reviver': function () { throw new Error('Function reviver is disabled') },