From 5662647f7208a08001424573a51a1e27d08dc84a Mon Sep 17 00:00:00 2001 From: Alex Brasetvik Date: Wed, 16 Oct 2019 02:06:24 +0200 Subject: [PATCH 1/2] Add _.getOwnValue, getValue that avoids prototype --- fp/_mapping.js | 1 + lodash.js | 14 ++++++++++++++ test/test.js | 29 ++++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/fp/_mapping.js b/fp/_mapping.js index a642ec0584..05e202613c 100644 --- a/fp/_mapping.js +++ b/fp/_mapping.js @@ -87,6 +87,7 @@ exports.aryMethod = { 'every', 'filter', 'find', 'findIndex', 'findKey', 'findLast', 'findLastIndex', 'findLastKey', 'flatMap', 'flatMapDeep', 'flattenDepth', 'forEach', 'forEachRight', 'forIn', 'forInRight', 'forOwn', 'forOwnRight', 'get', + 'getOwnValue', 'groupBy', 'gt', 'gte', 'has', 'hasIn', 'includes', 'indexOf', 'intersection', 'invertBy', 'invoke', 'invokeMap', 'isEqual', 'isMatch', 'join', 'keyBy', 'lastIndexOf', 'lt', 'lte', 'map', 'mapKeys', 'mapValues', 'matchesProperty', diff --git a/lodash.js b/lodash.js index d8b959a17f..ae984afd5c 100644 --- a/lodash.js +++ b/lodash.js @@ -1129,6 +1129,19 @@ return object == null ? undefined : object[key]; } + /** + * Gets the value at `key` of `object`, if it's an "own property" and + * not coming from the object's prototype. + * + * @private + * @param {Object} [object] The object to query. + * @param {string} key The key of the property to get. + * @returns {*} Returns the property value if it's the object's own property. + */ + function getOwnValue(object, key) { + return object == null ? undefined : (hasOwnProperty.call(object, key) && object[key] || undefined) + } + /** * Checks if `string` contains Unicode symbols. * @@ -16762,6 +16775,7 @@ lodash.forOwn = forOwn; lodash.forOwnRight = forOwnRight; lodash.get = get; + lodash.getOwnValue = getOwnValue; lodash.gt = gt; lodash.gte = gte; lodash.has = has; diff --git a/test/test.js b/test/test.js index fed616b280..ac41eb4565 100644 --- a/test/test.js +++ b/test/test.js @@ -1489,6 +1489,33 @@ /*--------------------------------------------------------------------------*/ + QUnit.module('lodash.getOwnValue'); + + (function() { + + var obj = {'foo': 'bar', 'falsy-0': 0, 'falsy-false': false}; + obj.__proto__ = {'sourceURL': 'does not belong to a'}; + + QUnit.test('should return `undefined` for keys that only exist in the prototype', function (assert) { + assert.expect(1); + assert.equal(_.getOwnValue(obj, 'sourceURL'), undefined); + }); + + QUnit.test('should return values that exist', function (assert) { + assert.expect(1); + assert.equal(_.getOwnValue(obj, 'foo'), 'bar'); + }); + + QUnit.test('should return falsy values properly', function (assert) { + assert.expect(2); + assert.equal(_.getOwnValue(obj, 'falsy-0'), 0); + assert.equal(_.getOwnValue(obj, 'falsy-false'), false); + }); + + }); + + /*--------------------------------------------------------------------------*/ + QUnit.module('lodash.at'); (function() { @@ -26851,7 +26878,7 @@ var acceptFalsey = lodashStable.difference(allMethods, rejectFalsey); QUnit.test('should accept falsey arguments', function(assert) { - assert.expect(316); + assert.expect(317); var arrays = lodashStable.map(falsey, stubArray); From 19ad6bc39f05bbb4fba9edd56d83f1bc402ed87d Mon Sep 17 00:00:00 2001 From: Alex Brasetvik Date: Wed, 16 Oct 2019 02:08:41 +0200 Subject: [PATCH 2/2] Avoid prototype's values in _.template options --- lodash.js | 29 ++++++++++++++++++----------- test/test.js | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/lodash.js b/lodash.js index ae984afd5c..b90e9c5eed 100644 --- a/lodash.js +++ b/lodash.js @@ -1139,7 +1139,9 @@ * @returns {*} Returns the property value if it's the object's own property. */ function getOwnValue(object, key) { - return object == null ? undefined : (hasOwnProperty.call(object, key) && object[key] || undefined) + return object == null + ? undefined + : (hasOwnProperty.call(object, key) ? object[key] : undefined) } /** @@ -14810,10 +14812,16 @@ options = undefined; } string = toString(string); + /* These variables are particularly sensitive to prototype pollution. + Look them up prior to assignInWith, which will merge in anything + already in the prototype. */ + var sourceURL = getOwnValue(options, 'sourceURL') || getOwnValue(settings, 'sourceURL'), + variable = getOwnValue(options, 'variable') || getOwnValue(settings, 'variable'), + imports = getOwnValue(options, 'imports') || getOwnValue(settings, 'imports'); + options = assignInWith({}, options, settings, customDefaultsAssignIn); - var imports = assignInWith({}, options.imports, settings.imports, customDefaultsAssignIn), - importsKeys = keys(imports), + var importsKeys = keys(imports), importsValues = baseValues(imports, importsKeys); var isEscaping, @@ -14832,11 +14840,13 @@ // Use a sourceURL for easier debugging. // The sourceURL gets injected into the source that's eval-ed, so be careful - // with lookup (in case of e.g. prototype pollution), and strip newlines if any. - // A newline wouldn't be a valid sourceURL anyway, and it'd enable code injection. - var sourceURL = '//# sourceURL=' + - (hasOwnProperty.call(options, 'sourceURL') - ? (options.sourceURL + '').replace(/[\r\n]/g, ' ') + // with lookup (in case of e.g. prototype pollution), and normalize all kinds of + // whitespace, so e.g. newlines (and unicode versions of it) can't sneak in. + // While variable is also injected below, it is less likely to be sourced from + // somewhere not trustworthy. + sourceURL = '//# sourceURL=' + ( + sourceURL + ? (sourceURL + '').replace(/[\s]/g, ' ') : ('lodash.templateSources[' + (++templateCounter) + ']') ) + '\n'; @@ -14869,9 +14879,6 @@ // If `variable` is not specified wrap a with-statement around the generated // code to add the data object to the top of the scope chain. - // Like with sourceURL, we take care to not check the option's prototype, - // as this configuration is a code injection vector. - var variable = hasOwnProperty.call(options, 'variable') && options.variable; if (!variable) { source = 'with (obj) {\n' + source + '\n}\n'; } diff --git a/test/test.js b/test/test.js index ac41eb4565..2e74c40025 100644 --- a/test/test.js +++ b/test/test.js @@ -22652,6 +22652,27 @@ assert.deepEqual(actual, expected); }); + QUnit.test('should not let a polluted prototype affect generated code', function(assert) { + assert.expect(1); + + // Intentionally pollute the prototype. These will cause a compilation error if part of the code + Object.prototype['sourceURL'] = '\u2028\n!err, please!'; + Object.prototype['variable'] = '}{!?'; + Object.prototype['imports'] = {',),': ''}; + + var actual, + expected = 'no error'; + try { + actual = _.template(expected)(); + } catch (e) {} + + delete Object.prototype['sourceURL']; + delete Object.prototype['variable']; + delete Object.prototype['imports']; + + assert.equal(actual, expected); + }); + QUnit.test('should work as an iteratee for methods like `_.map`', function(assert) { assert.expect(1);