Skip to content

Commit

Permalink
Fixed a security vulnerability in the expression parser allowing exec…
Browse files Browse the repository at this point in the history
…ution of arbitrary JavaScript
  • Loading branch information
josdejong committed Mar 31, 2017
1 parent 691d555 commit 2f45600
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 4 deletions.
6 changes: 6 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# History


## 2017-03-31, version 3.10.2

- Fixed a security vulnerability in the expression parser allowing
execution of arbitrary JavaScript. Thanks @CapacitorSet and @denvit.


## 2017-03-26, version 3.10.1

- Fixed `xgcd` for negative values. Thanks @litmit.
Expand Down
2 changes: 2 additions & 0 deletions docs/expressions/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ This section is divided in the following pages:
- [Algebra](algebra.md) describing symbolic computation in math.js.
- [Customization](customization.md) describes how to customize processing and
evaluation of expressions.
- [Security](security.md) about security risks of executing arbitrary
expressions.
42 changes: 42 additions & 0 deletions docs/expressions/security.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Security

Executing arbitrary expressions like enabled by the expression parser of
mathjs involves a risk in general. When you're using mathjs to let users
execute arbitrary expressions, it's good to take a moment to think about
possible security and stability implications.

## Security risks

A user could try to inject malicious JavaScript code via the expression
parser. The expression parser of mathjs offers a sandboxed environment
to execute expressions which should make this impossible. It's possible
though that there is an unknown security hole, so it's important to be
careful, especially when allowing server side execution of arbitrary
expressions.

The expression parser of mathjs parses the input in a controlled
way into an expression tree, then compiles it into fast performing
JavaScript using JavaScript's `eval` before actually evaluating the
expression. The parser actively prevents access to JavaScripts internal
`eval` and `new Function` which are the main cause of security attacks.

When running a node.js server, it's good to be aware of the different
types of security risks. The risk whe running inside a browser may be
limited though it's good to be aware of [Cross side scripting (XSS)](https://www.wikiwand.com/en/Cross-site_scripting) vulnerabilities. A nice overview of
security risks of a node.js servers is listed in an article [Node.js security checklist](https://blog.risingstack.com/node-js-security-checklist/) by Gergely Nemeth.
Lastly, one could look into running server side code in a sandboxed
[node.js VM](https://nodejs.org/api/vm.html).

## Stability risks

A user could accidentally or on purpose execute a
heavy expression like creating a huge matrix. That can let the
JavaScript engine run out of memory or freeze it when the CPU goes
to 100% for a long time.

To protect against this sort of issue, one can run the expression parser
in a separate Web Worker or child_process, so it can't affect the
main process. The workers can be killed when it runs for too
long or consumes too much memory. A useful library in this regard
is [workerpool](https://github.com/josdejong/workerpool), which makes
it easy to manage a pool of workers in both browser and node.js.
3 changes: 3 additions & 0 deletions lib/expression/function/eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ function factory (type, config, load, typed) {
/**
* Evaluate an expression.
*
* Note the evaluating arbitrary expressions may involve security risks,
* see [http://mathjs.org/docs/expressions/security.html](http://mathjs.org/docs/expressions/security.html) for more information.
*
* Syntax:
*
* math.eval(expr)
Expand Down
3 changes: 3 additions & 0 deletions lib/expression/function/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ function factory (type, config, load, typed) {
* Parse an expression. Returns a node tree, which can be evaluated by
* invoking node.eval();
*
* Note the evaluating arbitrary expressions may involve security risks,
* see [http://mathjs.org/docs/expressions/security.html](http://mathjs.org/docs/expressions/security.html) for more information.
*
* Syntax:
*
* math.parse(expr)
Expand Down
34 changes: 31 additions & 3 deletions lib/expression/node/FunctionNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ function factory (type, config, load, typed, math) {
* @private
*/
FunctionNode.prototype._compile = function (defs, args) {
defs.customs = customs;

// compile fn and arguments
var jsFn = this.fn._compile(defs, args);
var jsArgs = this.args.map(function (arg) {
Expand All @@ -89,11 +91,11 @@ function factory (type, config, load, typed, math) {
argsName = this._getUniqueArgumentsName(defs);
defs[argsName] = this.args;

return jsFn + '(' + argsName + ', math, ' + jsScope + ')';
return 'customs(' + jsFn + ')(' + argsName + ', math, ' + jsScope + ')';
}
else {
// "regular" evaluation
return jsFn + '(' + jsArgs.join(', ') + ')';
return 'customs(' + jsFn + ')(' + jsArgs.join(', ') + ')';
}
}
else if (this.fn.isAccessorNode && this.fn.index.isObjectProperty()) {
Expand All @@ -106,6 +108,7 @@ function factory (type, config, load, typed, math) {

return '(function () {' +
'var object = ' + jsObject + ';' +
'customs(object["' + prop + '"]);' +
'return (object["' + prop + '"] && object["' + prop + '"].rawArgs) ' +
' ? object["' + prop + '"](' + argsName + ', math, ' + jsScope + ')' +
' : object["' + prop + '"](' + jsArgs.join(', ') + ')' +
Expand All @@ -117,7 +120,7 @@ function factory (type, config, load, typed, math) {
defs[argsName] = this.args;

return '(function () {' +
'var fn = ' + jsFn + ';' +
'var fn = customs(' + jsFn + ');' +
'return (fn && fn.rawArgs) ' +
' ? fn(' + argsName + ', math, ' + jsScope + ')' +
' : fn(' + jsArgs.join(', ') + ')' +
Expand Down Expand Up @@ -414,6 +417,31 @@ function factory (type, config, load, typed, math) {
return FunctionNode;
}

/**
* Don't allow access to Function and eval, which can be used
* to execute arbitrary JavaScript code. Example of this vulnerability:
*
* math.eval('[].map.constructor("console.log(\\"hacked...\\")")()')
*
* @param {function} fn
* @return {function} Returns the input function when ok,
* else throws an error
*/
function customs (fn) {
if (fn === Function) {
throw new Error('Calling "Function" is not allowed');
}

if (fn === jsEval) {
throw new Error('Calling "eval" is not allowed');
}

// this function is ok
return fn;
}

var jsEval = eval

exports.name = 'FunctionNode';
exports.path = 'expression.node';
exports.math = true; // request access to the math namespace as 5th argument of the factory function
Expand Down
2 changes: 1 addition & 1 deletion lib/expression/node/utils/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function factory (type, config, load, typed) {
}
else if (typeof object === 'object') {
if (!index.isObjectProperty()) {
throw TypeError('Cannot apply a numeric index as object property');
throw new TypeError('Cannot apply a numeric index as object property');
}
return object[index.getObjectProperty()];
}
Expand Down
23 changes: 23 additions & 0 deletions test/expression/parse.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2041,4 +2041,27 @@ describe('parse', function() {

});

describe('security', function () {

it ('should not allow calling Function from an object property', function () {
assert.throws(function () {
console.log(math.eval('[].map.constructor("console.log(\\"hacked...\\")")()'))
math.eval('[].map.constructor("console.log(\\"hacked...\\")")()')
}, /Error: Calling "Function" is not allowed/)
})

it ('should not allow calling Function', function () {
assert.throws(function () {
math.eval('disguised("console.log(\\"hacked...\\")")()', {disguised: Function})
}, /Error: Calling "Function" is not allowed/)
})

it ('should not allow calling eval', function () {
assert.throws(function () {
math.eval('disguised("console.log(\\"hacked...\\")")()', {disguised: eval})
}, /Error: Calling "eval" is not allowed/)
})

});

});

0 comments on commit 2f45600

Please sign in to comment.