Skip to content

Commit

Permalink
Fix possible code execution in (already unsafe) load()
Browse files Browse the repository at this point in the history
... when object with executable toString() property is used as a map key
  • Loading branch information
rlidwka committed Apr 5, 2019
1 parent 9d4ce5e commit e18afbf
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 2 deletions.
19 changes: 17 additions & 2 deletions lib/js-yaml/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ var PATTERN_TAG_HANDLE = /^(?:!|!!|![a-z\-]+!)$/i;
var PATTERN_TAG_URI = /^(?:!|[^,\[\]\{\}])(?:%[0-9a-f]{2}|[0-9a-z\-#;\/\?:@&=\+\$,_\.!~\*'\(\)\[\]])*$/i;


function _class(obj) { return Object.prototype.toString.call(obj); }

function is_EOL(c) {
return (c === 0x0A/* LF */) || (c === 0x0D/* CR */);
}
Expand Down Expand Up @@ -287,16 +289,29 @@ function storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valu

// The output is a plain object here, so keys can only be strings.
// We need to convert keyNode to a string, but doing so can hang the process
// (deeply nested arrays that explode exponentially using aliases) or execute
// code via toString.
// (deeply nested arrays that explode exponentially using aliases).
if (Array.isArray(keyNode)) {
keyNode = Array.prototype.slice.call(keyNode);

for (index = 0, quantity = keyNode.length; index < quantity; index += 1) {
if (Array.isArray(keyNode[index])) {
throwError(state, 'nested arrays are not supported inside keys');
}

if (typeof keyNode === 'object' && _class(keyNode[index]) === '[object Object]') {
keyNode[index] = '[object Object]';
}
}
}

// Avoid code execution in load() via toString property
// (still use its own toString for arrays, timestamps,
// and whatever user schema extensions happen to have @@toStringTag)
if (typeof keyNode === 'object' && _class(keyNode) === '[object Object]') {
keyNode = '[object Object]';
}


keyNode = String(keyNode);

if (_result === null) {
Expand Down
1 change: 1 addition & 0 deletions test/issues/0480-date.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ !<tag:yaml.org,2002:timestamp> '2019-04-05T12:00:43.467Z': 123 }
4 changes: 4 additions & 0 deletions test/issues/0480-fn-array.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
? [
123,
{ toString: !<tag:yaml.org,2002:js/function> 'function (){throw new Error("code execution")}' }
] : key
1 change: 1 addition & 0 deletions test/issues/0480-fn.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ toString: !<tag:yaml.org,2002:js/function> 'function (){throw new Error("code execution")}' } : key
1 change: 1 addition & 0 deletions test/issues/0480-fn2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ __proto__: { toString: !<tag:yaml.org,2002:js/function> 'function(){throw new Error("code execution")}' } } : key
34 changes: 34 additions & 0 deletions test/issues/0480.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';


var assert = require('assert');
var yaml = require('../../');
var readFileSync = require('fs').readFileSync;


test('Should not execute code when object with toString property is used as a key', function () {
var data = yaml.load(readFileSync(require('path').join(__dirname, '/0480-fn.yml'), 'utf8'));

assert.deepEqual(data, { '[object Object]': 'key' });
});

test('Should not execute code when object with __proto__ property is used as a key', function () {
var data = yaml.load(readFileSync(require('path').join(__dirname, '/0480-fn2.yml'), 'utf8'));

assert.deepEqual(data, { '[object Object]': 'key' });
});

test('Should not execute code when object inside array is used as a key', function () {
var data = yaml.load(readFileSync(require('path').join(__dirname, '/0480-fn-array.yml'), 'utf8'));

assert.deepEqual(data, { '123,[object Object]': 'key' });
});

// this test does not guarantee in any way proper handling of date objects,
// it just keeps old behavior whenever possible
test('Should leave non-plain objects as is', function () {
var data = yaml.load(readFileSync(require('path').join(__dirname, '/0480-date.yml'), 'utf8'));

assert.deepEqual(Object.keys(data).length, 1);
assert(/2019/.test(Object.keys(data)[0]));
});

0 comments on commit e18afbf

Please sign in to comment.