Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible code execution in (already unsafe) load() #480

Merged
merged 1 commit into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]));
});