Skip to content

Commit

Permalink
Merge pull request #480 from nodeca/toString
Browse files Browse the repository at this point in the history
Fix possible code execution in (already unsafe) load()
  • Loading branch information
Vitaly Puzrin authored Apr 5, 2019
2 parents 9d4ce5e + e18afbf commit b2f9e88
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 b2f9e88

Please sign in to comment.