fix: security vulnerability in FunctionNode and SymbolNode allowi…

…ng arbitrary code execution via `math.evaluate`

HISTORY.md

@@ -2,6 +2,8 @@
 
 # unpublished changes since 11.9.0
 
+- Fix a security vulnerability in `FunctionNode` and `SymbolNode` allowing
+  arbitrary code execution via `math.evaluate`. Thanks Harry Chen.
 - Fix #3001: mathjs bundle containing `new Function(...)` (CSP issue).
 
 

src/expression/node/FunctionNode.js

@@ -1,7 +1,7 @@
 import { isAccessorNode, isFunctionAssignmentNode, isIndexNode, isNode, isSymbolNode } from '../../utils/is.js'
 import { escape, format } from '../../utils/string.js'
 import { hasOwnProperty } from '../../utils/object.js'
-import { getSafeProperty, validateSafeMethod } from '../../utils/customs.js'
+import { getSafeProperty, getSafeMethod } from '../../utils/customs.js'
 import { createSubScope } from '../../utils/scope.js'
 import { factory } from '../../utils/factory.js'
 import { defaultTemplate, latexFunctions } from '../../utils/latex.js'
@@ -205,7 +205,7 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
         } else { // the function symbol is an argName
           const rawArgs = this.args
           return function evalFunctionNode (scope, args, context) {
-            const fn = args[name]
+            const fn = getSafeProperty(args, name)
             if (typeof fn !== 'function') {
               throw new TypeError(
                 `Argument '${name}' was not a function; received: ${strin(fn)}`
@@ -235,18 +235,15 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
 
         return function evalFunctionNode (scope, args, context) {
           const object = evalObject(scope, args, context)
-          validateSafeMethod(object, prop)
-          const isRaw = object[prop] && object[prop].rawArgs
+          const fn = getSafeMethod(object, prop)
 
-          if (isRaw) {
+          if (fn?.rawArgs) {
             // "Raw" evaluation
-            return object[prop](
-              rawArgs, math, createSubScope(scope, args), scope)
+            return fn(rawArgs, math, createSubScope(scope, args), scope)
           } else {
             // "regular" evaluation
-            const values = evalArgs.map(
-              (evalArg) => evalArg(scope, args, context))
-            return object[prop].apply(object, values)
+            const values = evalArgs.map((evalArg) => evalArg(scope, args, context))
+            return fn.apply(object, values)
           }
         }
       } else {

src/expression/node/ObjectNode.js

@@ -1,8 +1,8 @@
+import { getSafeProperty } from '../../utils/customs.js'
+import { factory } from '../../utils/factory.js'
 import { isNode } from '../../utils/is.js'
-import { escape, stringify } from '../../utils/string.js'
-import { isSafeProperty } from '../../utils/customs.js'
 import { hasOwnProperty } from '../../utils/object.js'
-import { factory } from '../../utils/factory.js'
+import { escape, stringify } from '../../utils/string.js'
 
 const name = 'ObjectNode'
 const dependencies = [
@@ -58,11 +58,9 @@ export const createObjectNode = /* #__PURE__ */ factory(name, dependencies, ({ N
           // so you cannot create a key like {"co\\u006Estructor": null}
           const stringifiedKey = stringify(key)
           const parsedKey = JSON.parse(stringifiedKey)
-          if (!isSafeProperty(this.properties, parsedKey)) {
-            throw new Error('No access to property "' + parsedKey + '"')
-          }
+          const prop = getSafeProperty(this.properties, key)
 
-          evalEntries[parsedKey] = this.properties[key]._compile(math, argNames)
+          evalEntries[parsedKey] = prop._compile(math, argNames)
         }
       }
 

src/expression/node/SymbolNode.js

@@ -62,7 +62,7 @@ export const createSymbolNode = /* #__PURE__ */ factory(name, dependencies, ({ m
         // (like an x when inside the expression of a function
         // assignment `f(x) = ...`)
         return function (scope, args, context) {
-          return args[name]
+          return getSafeProperty(args, name)
         }
       } else if (name in math) {
         return function (scope, args, context) {

src/utils/customs.js

@@ -88,12 +88,14 @@ function isSafeProperty (object, prop) {
  * Throws an error when that's not the case.
  * @param {Object} object
  * @param {string} method
+ * @return {function} Returns the method when valid
  */
-// TODO: merge this function into assign.js?
-function validateSafeMethod (object, method) {
+function getSafeMethod (object, method) {
   if (!isSafeMethod(object, method)) {
     throw new Error('No access to method "' + method + '"')
   }
+
+  return object[method]
 }
 
 /**
@@ -158,6 +160,6 @@ export { setSafeProperty }
 export { isSafeProperty }
 export { hasSafeProperty }
 export { getSafeProperties }
-export { validateSafeMethod }
+export { getSafeMethod }
 export { isSafeMethod }
 export { isPlainObject }

test/unit-tests/expression/security.test.js

@@ -362,6 +362,16 @@ describe('security', function () {
     assert.throws(function () { math.evaluate('unit("5cm").valueOf') }, /Cannot access method "valueOf" as a property/)
   })
 
+  it('should not allow accessing constructor via FunctionNode.name', function () {
+    assert.throws(function () {
+      // could execute a nodejs script like "return process.mainModule.require(child_process).execSync(whoami)"
+      const result = math.evaluate('evalFunctionNode=parse("constructor(\'return process.version\')")._compile({},{});' +
+        'f=evalFunctionNode(null,cos);' +
+        'f()')
+      console.warn('Hacked! node.js version:', result.entries[0])
+    }, /No access to property "constructor"/)
+  })
+
   it('should not have access to specific namespaces', function () {
     Object.keys(math.expression.mathWithTransform).forEach(function (name) {
       const value = math.expression.mathWithTransform[name]

test/unit-tests/utils/customs.test.js

@@ -1,7 +1,13 @@
 // test boolean utils
 import assert from 'assert'
 
-import { isPlainObject, isSafeMethod, isSafeProperty } from '../../../src/utils/customs.js'
+import {
+  getSafeMethod,
+  getSafeProperty,
+  isPlainObject,
+  isSafeMethod,
+  isSafeProperty
+} from '../../../src/utils/customs.js'
 import math from '../../../src/defaultInstance.js'
 
 describe('customs', function () {
@@ -110,6 +116,20 @@ describe('customs', function () {
     })
   })
 
+  describe('getSafeMethod', function () {
+    it('should return a method when safe', function () {
+      const obj = { getName: () => 'Joe' }
+
+      assert.strictEqual(getSafeMethod(obj, 'getName'), obj.getName)
+    })
+
+    it('should throw an exception when a method is unsafe', function () {
+      assert.throws(() => {
+        getSafeMethod(Function, 'constructor')
+      }, /Error: No access to method "constructor"/)
+    })
+  })
+
   describe('isSafeProperty', function () {
     it('should test properties on plain objects', function () {
       const object = {}
@@ -160,6 +180,32 @@ describe('customs', function () {
     })
   })
 
+  describe('getSafeProperty', function () {
+    it('should return a method when safe', function () {
+      const obj = { getName: () => 'Joe' }
+
+      assert.strictEqual(getSafeProperty(obj, 'getName'), obj.getName)
+    })
+
+    it('should return a property when safe', function () {
+      const obj = { username: 'Joe' }
+
+      assert.strictEqual(getSafeProperty(obj, 'username'), 'Joe')
+    })
+
+    it('should throw an exception when a method is unsafe', function () {
+      assert.throws(() => {
+        getSafeProperty(Function, 'constructor')
+      }, /Error: No access to property "constructor"/)
+    })
+
+    it('should throw an exception when a property is unsafe', function () {
+      assert.throws(() => {
+        getSafeProperty({ constructor: 'test' }, 'constructor')
+      }, /Error: No access to property "constructor"/)
+    })
+  })
+
   it('should distinguish plain objects', function () {
     const a = {}
     const b = Object.create(a)