diff --git a/lib/core/function/import.js b/lib/core/function/import.js index 2ed292254..0cd81d779 100644 --- a/lib/core/function/import.js +++ b/lib/core/function/import.js @@ -145,14 +145,14 @@ function factory (type, config, load, typed, math) { function _importTransform (name, value) { if (value && typeof value.transform === 'function') { math.expression.transform[name] = value.transform; - if (!unsafe.hasOwnProperty(name)) { + if (allowedInExpressions(name)) { math.expression.mathWithTransform[name] = value.transform } } else { // remove existing transform delete math.expression.transform[name] - if (!unsafe.hasOwnProperty(name)) { + if (allowedInExpressions(name)) { math.expression.mathWithTransform[name] = value } } @@ -227,7 +227,7 @@ function factory (type, config, load, typed, math) { lazy(namespace, name, resolver); if (!existingTransform) { - if (!unsafe.hasOwnProperty(name)) { + if (factory.path === 'expression.transform' || factoryAllowedInExpressions(factory)) { lazy(math.expression.mathWithTransform, name, resolver); } } @@ -236,7 +236,7 @@ function factory (type, config, load, typed, math) { namespace[name] = resolver(); if (!existingTransform) { - if (!unsafe.hasOwnProperty(name)) { + if (factory.path === 'expression.transform' || factoryAllowedInExpressions(factory)) { math.expression.mathWithTransform[name] = resolver(); } } @@ -280,10 +280,19 @@ function factory (type, config, load, typed, math) { return typeof fn === 'function' && typeof fn.signatures === 'object'; } + function allowedInExpressions (name) { + return !unsafe.hasOwnProperty(name); + } + + function factoryAllowedInExpressions (factory) { + return factory.path === undefined && !unsafe.hasOwnProperty(factory.name); + } + // namespaces and functions not available in the parser for safety reasons var unsafe = { 'expression': true, 'type': true, + 'docs': true, 'error': true, 'json': true, 'chain': true // chain method not supported. Note that there is a unit chain too. diff --git a/lib/utils/customs.js b/lib/utils/customs.js index 1b6a122ab..c54e0662b 100644 --- a/lib/utils/customs.js +++ b/lib/utils/customs.js @@ -100,18 +100,20 @@ function isSafeMethod (object, method) { return isPlainObject(object); } else { - // only allow methods defined on the prototype of this object - // and not defined on the prototype of native Object - // i.e. constructor, __defineGetter__, hasOwnProperty, etc. are not allowed - // A few safe native methods are allowed: toString, valueOf, toLocaleString - return (object && - hasOwnProperty(object.constructor.prototype, method) && + // only allow methods: + // - defined on the prototype of this object + // - not defined on the prototype of native Object + // i.e. constructor, __defineGetter__, hasOwnProperty, etc. are not allowed + // - calling methods on a function (like bind) is not allowed + // - A few safe native methods are allowed: toString, valueOf, toLocaleString + return (object && typeof object !== 'function' && + (hasOwnProperty(object.constructor.prototype, method) || + hasOwnProperty(object.__proto__, method)) && (!hasOwnProperty(Object.prototype, method) || hasOwnProperty(safeNativeMethods, method))); } } function isPlainObject (object) { - // TODO: improve this function return typeof object === 'object' && object && object.constructor === Object; } @@ -126,3 +128,4 @@ exports.setSafeProperty = setSafeProperty; exports.isSafeProperty = isSafeProperty; exports.validateSafeMethod = validateSafeMethod; exports.isSafeMethod = isSafeMethod; +exports.isPlainObject = isPlainObject; diff --git a/test/expression/parse.test.js b/test/expression/parse.test.js index 746c7a68f..2c3489595 100644 --- a/test/expression/parse.test.js +++ b/test/expression/parse.test.js @@ -820,8 +820,9 @@ describe('parse', function() { assert.deepEqual(parseAndEval('bignumber(2)["plus"](3)'), math.bignumber(5)); }); - it('should invoke toString on some object', function () { + it('should invoke native methods on a number', function () { assert.strictEqual(parseAndEval('(3).toString()'), '3'); + assert.strictEqual(parseAndEval('(3.2).toFixed()'), '3'); }); it('should get nested object property with mixed dot- and index-notation', function () { diff --git a/test/expression/security.test.js b/test/expression/security.test.js index a343b3e26..7edb4d2b1 100644 --- a/test/expression/security.test.js +++ b/test/expression/security.test.js @@ -286,6 +286,10 @@ describe('security', function () { assert.equal(math.eval('sqrt(4)'), 2); }) + it ('should allow invoking methods on complex numbers', function () { + assert.deepEqual(math.eval('complex(4, 0).sqrt(2)'), math.complex(2, 0)); + }) + it ('should allow accessing properties on an object', function () { assert.deepEqual(math.eval('obj.a', {obj: {a:42}}), 42); }) @@ -296,19 +300,56 @@ describe('security', function () { }, /Error: No access to property "constructor"/) }) + it ('should not allow accessing __proto__', function () { + assert.throws(function () { + math.eval('{}.__proto__'); + }, /Error: No access to property "__proto__"/) + }) + it ('should not allow getting properties from non plain objects', function () { assert.throws(function () {math.eval('[]._data')}, /No access to property "_data"/) assert.throws(function () {math.eval('unit("5cm").valueOf')}, /Cannot access method "valueOf" as a property/); }); it ('should not have access to specific namespaces', function () { + Object.keys(math.expression.mathWithTransform).forEach (function (name) { + var value = math.expression.mathWithTransform[name]; + + // only plain functions allowed, no constructor functions + if (typeof value === 'function') { + assert.strictEqual(isPlainFunction(value), true, + 'only plain functions expected, constructor functions not allowed (name: ' + name + ')'); + } + else { + // plain objects not allowed, only class instances like units and complex numbers + if (value && typeof value === 'object') { + if (isPlainObject(value) && (name !== 'uninitialized' )) { + throw new Error('plain objects are not allowed, only class instances (object name: ' + name + ')'); + } + } + } + + }); + assert.throws(function () {math.eval('expression')}, /Undefined symbol/); assert.throws(function () {math.eval('type')}, /Undefined symbol/); assert.throws(function () {math.eval('error')}, /Undefined symbol/); assert.throws(function () {math.eval('json')}, /Undefined symbol/); + assert.strictEqual(math.expression.mathWithTransform.Matrix, undefined); + assert.strictEqual(math.expression.mathWithTransform.Node, undefined); assert.strictEqual(math.expression.mathWithTransform.chain, undefined); assert.deepEqual(math.eval('chain'), math.unit('chain')); }); }); + +function isPlainObject (object) { + return typeof object === 'object' && object && + object.constructor === Object && + object.__proto__ === Object.prototype; +} + +function isPlainFunction (fn) { + return typeof fn === 'function' && fn.prototype.constructor === fn; +} diff --git a/test/utils/customs.test.js b/test/utils/customs.test.js new file mode 100644 index 000000000..a412222cc --- /dev/null +++ b/test/utils/customs.test.js @@ -0,0 +1,85 @@ +// test boolean utils +var assert = require('assert'); +var approx = require('../../tools/approx'); +var customs = require('../../lib/utils/customs'); +var math = require('../../index'); + +describe ('customs', function () { + + describe ('isSafeMethod', function() { + + it ('plain objects', function () { + var object = { + fn: function () {} + } + assert.equal(customs.isSafeMethod(object, 'fn'), true); + assert.equal(customs.isSafeMethod(object, 'toString'), true); + assert.equal(customs.isSafeMethod(object, 'toLocaleString'), true); + assert.equal(customs.isSafeMethod(object, 'valueOf'), true); + + assert.equal(customs.isSafeMethod(object, 'constructor'), false); + assert.equal(customs.isSafeMethod(object, 'hasOwnProperty'), false); + assert.equal(customs.isSafeMethod(object, 'isPrototypeOf'), false); + assert.equal(customs.isSafeMethod(object, 'propertyIsEnumerable'), false); + assert.equal(customs.isSafeMethod(object, '__defineGetter__'), false); + assert.equal(customs.isSafeMethod(object, '__defineSetter__'), false); + assert.equal(customs.isSafeMethod(object, '__lookupGetter__'), false); + assert.equal(customs.isSafeMethod(object, '__lookupSetter__'), false); + + // non existing method + assert.equal(customs.isSafeMethod(object, 'foo'), false); + }); + + it ('function objects', function () { + var f = function () {}; + + assert.equal(customs.isSafeMethod(f, 'call'), false); + assert.equal(customs.isSafeMethod(f, 'bind'), false); + }); + + it ('classes', function () { + var matrix = math.matrix(); + assert.equal(customs.isSafeMethod(matrix, 'get'), true); + assert.equal(customs.isSafeMethod(matrix, 'toString'), true); + + var complex = math.complex(); + assert.equal(customs.isSafeMethod(complex, 'sqrt'), true); + assert.equal(customs.isSafeMethod(complex, 'toString'), true); + + var unit = math.unit('5cm'); + assert.equal(customs.isSafeMethod(unit, 'toNumeric'), true); + assert.equal(customs.isSafeMethod(unit, 'toString'), true); + + // extend the class instance with an overridden method + matrix.myFunction = function () {}; + assert.equal(customs.isSafeMethod(matrix, 'myFunction'), false); + + // unsafe native methods + assert.equal(customs.isSafeMethod(matrix, 'constructor'), false); + assert.equal(customs.isSafeMethod(matrix, 'hasOwnProperty'), false); + assert.equal(customs.isSafeMethod(matrix, 'isPrototypeOf'), false); + assert.equal(customs.isSafeMethod(matrix, 'propertyIsEnumerable'), false); + assert.equal(customs.isSafeMethod(matrix, '__defineGetter__'), false); + assert.equal(customs.isSafeMethod(matrix, '__defineSetter__'), false); + assert.equal(customs.isSafeMethod(matrix, '__lookupGetter__'), false); + assert.equal(customs.isSafeMethod(matrix, '__lookupSetter__'), false); + + // non existing method + assert.equal(customs.isSafeMethod(matrix, 'nonExistingMethod'), false); + }); + + }); + + it ('should distinguish plain objects', function () { + var a = {}; + var b = Object.create(a); + assert.equal(customs.isPlainObject (a), true); + assert.equal(customs.isPlainObject (b), true); + + assert.equal(customs.isPlainObject (math.unit('5cm')), false); + assert.equal(customs.isPlainObject (math.unit('5cm')), false); + assert.equal(customs.isPlainObject ([]), false); + // assert.equal(customs.isPlainObject (math.complex()), false); // FIXME: shouldn't treat Complex as a plain object (it is a plain object which has __proto__ overridden) + assert.equal(customs.isPlainObject (math.matrix()), false); + }); +}); \ No newline at end of file