Skip to content

Commit

Permalink
Fixed a security issue where forbidden properties like constructor co…
Browse files Browse the repository at this point in the history
…uld be replaced by using unicode characters when creating an object
  • Loading branch information
josdejong committed Nov 18, 2017
1 parent 8d2d48d commit a60f3c8
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 3 deletions.
3 changes: 3 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
- Fixed a security issue in `typed-function` allowing arbitrary code execution
in the JavaScript engine by creating a typed function with JavaScript code
in the name. Thanks Masato Kinugawa.
- Fixed a security issue where forbidden properties like constructor could be
replaced by using unicode characters when creating an object. No known exploit,
but could possibly allow arbitrary code execution. Thanks Masato Kinugawa.


## 2017-10-18, version 3.16.5
Expand Down
10 changes: 7 additions & 3 deletions lib/expression/node/ObjectNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,15 @@ function factory (type, config, load, typed) {
var entries = [];
for (var key in node.properties) {
if (hasOwnProperty(node.properties, key)) {
if (!isSafeProperty(node.properties, key)) {
throw new Error('No access to property "' + key + '"');
// we stringify/parse the key here to resolve unicode characters,
// so you cannot create a key like {"co\\u006Estructor": null}
var stringifiedKey = stringify(key)
var parsedKey = JSON.parse(stringifiedKey)
if (!isSafeProperty(node.properties, parsedKey)) {
throw new Error('No access to property "' + parsedKey + '"');
}

entries.push(stringify(key) + ': ' + compile(node.properties[key], defs, args));
entries.push(stringifiedKey + ': ' + compile(node.properties[key], defs, args));
}
}
return '{' + entries.join(', ') + '}';
Expand Down
17 changes: 17 additions & 0 deletions test/expression/security.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,23 @@ describe('security', function () {
}, /Error: No access to property "bind/);
})

it ('should not allow disguising forbidden properties with unicode characters', function () {
var scope = {
a: {}
};

assert.throws(function () { math.eval('a.co\u006Estructor', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a["co\\u006Estructor"]', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a.constructor', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a.constructor = 2', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a["constructor"] = 2', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a["co\\u006Estructor"] = 2', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a = {"constructor": 2}', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a = {constructor: 2}', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a = {"co\\u006Estructor": 2}', scope); }, /Error: No access to property "constructor"/);
assert.throws(function () { math.eval('a = {co\u006Estructor: 2}', scope); }, /Error: No access to property "constructor"/);
})

it ('should not allow calling Function via imported, overridden function', function () {
assert.throws(function () {
var math2 = math.create();
Expand Down
5 changes: 5 additions & 0 deletions test/utils/customs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ describe ('customs', function () {

// non existing method
assert.equal(customs.isSafeMethod(matrix, 'nonExistingMethod'), false);

// method with unicode chars
assert.equal(customs.isSafeMethod(matrix, 'co\u006Estructor'), false);
});

});
Expand Down Expand Up @@ -113,6 +116,8 @@ describe ('customs', function () {
// non existing property
assert.equal(customs.isSafeProperty(object, 'bar'), true);

// property with unicode chars
assert.equal(customs.isSafeProperty(object, 'co\u006Estructor'), false);
});

it ('should test inherited properties on plain objects ', function () {
Expand Down

0 comments on commit a60f3c8

Please sign in to comment.