From 423b99822577696ce71c9bb95397517cd4c8d11d Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Thu, 21 May 2020 00:10:43 -0700 Subject: [PATCH 1/3] Backport tests --- test/fixtures/config.json | 11 ++++++++++- test/yargs-parser.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/test/fixtures/config.json b/test/fixtures/config.json index 43376463..f8918f61 100644 --- a/test/fixtures/config.json +++ b/test/fixtures/config.json @@ -3,5 +3,14 @@ "z": 55, "foo": "baz", "version": "1.0.2", - "truthy": true + "truthy": true, + "toString": "method name", + "__proto__": { + "aaa": 99 + }, + "bar": { + "__proto__": { + "bbb": 100 + } + } } diff --git a/test/yargs-parser.js b/test/yargs-parser.js index 5b59c615..4826b919 100644 --- a/test/yargs-parser.js +++ b/test/yargs-parser.js @@ -410,6 +410,26 @@ describe('yargs-parser', function () { describe('config', function () { var jsonPath = path.resolve(__dirname, './fixtures/config.json') + // Patching for https://snyk.io/vuln/SNYK-JS-YARGSPARSER-560381 + it('should not pollute the prototype', function () { + const argv = parser(['--foo', 'bar'], { + alias: { + z: 'zoom' + }, + default: { + settings: jsonPath + }, + config: 'settings' + }) + + argv.should.have.property('herp', 'derp') + argv.should.have.property('zoom', 55) + argv.should.have.property('foo').and.deep.equal('bar') + + expect({}.bbb).to.equal(undefined) + expect({}.aaa).to.equal(undefined) + }) + // See: https://github.com/chevex/yargs/issues/12 it('should load options and values from default config if specified', function () { var argv = parser([ '--foo', 'bar' ], { @@ -2375,4 +2395,12 @@ describe('yargs-parser', function () { }) argv.a.should.deep.equal(['a.txt', 'b.txt']) }) + + // Patching for https://snyk.io/vuln/SNYK-JS-YARGSPARSER-560381 + it('should not pollute the prototype', function () { + parser(['-f.__proto__.foo', '99', '-x.y.__proto__.bar', '100', '--__proto__', '200']) + Object.keys({}.__proto__).length.should.equal(0) // eslint-disable-line + expect({}.foo).to.equal(undefined) + expect({}.bar).to.equal(undefined) + }) }) From 678e06aa14bd8eb82da50d03bfaf76f18bfea6b0 Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Thu, 21 May 2020 01:25:14 -0700 Subject: [PATCH 2/3] WIP --- index.js | 115 +++++++++++++++++++------------------------ test/yargs-parser.js | 2 +- 2 files changed, 52 insertions(+), 65 deletions(-) diff --git a/index.js b/index.js index b71faf58..00374e15 100644 --- a/index.js +++ b/index.js @@ -2,15 +2,16 @@ var camelCase = require('camelcase') var path = require('path') var tokenizeArgString = require('./lib/tokenize-arg-string') var util = require('util') +var _ = require('lodash') function parse (args, opts) { - if (!opts) opts = {} + if (!opts) opts = _.create(null) // allow a string argument to be passed in rather // than an argv array. args = tokenizeArgString(args) // aliases might have transitive relationships, normalize this. - var aliases = combineAliases(opts.alias || {}) - var configuration = assign({ + var aliases = combineAliases(opts.alias || _.create(null)) + var configuration = _.assign({ 'short-option-groups': true, 'camel-case-expansion': true, 'dot-notation': true, @@ -19,27 +20,27 @@ function parse (args, opts) { 'duplicate-arguments-array': true, 'flatten-duplicate-arrays': true }, opts.configuration) - var defaults = opts.default || {} + var defaults = opts.default || _.create(null) var configObjects = opts.configObjects || [] var envPrefix = opts.envPrefix - var newAliases = {} + var newAliases = _.create(null) // allow a i18n handler to be passed in, default to a fake one (util.format). var __ = opts.__ || function (str) { return util.format.apply(util, Array.prototype.slice.call(arguments)) } var error = null var flags = { - aliases: {}, - arrays: {}, - bools: {}, - strings: {}, - numbers: {}, - counts: {}, - normalize: {}, - configs: {}, - defaulted: {}, - nargs: {}, - coercions: {} + aliases: _.create(null), + arrays: _.create(null), + bools: _.create(null), + strings: _.create(null), + numbers: _.create(null), + counts: _.create(null), + normalize: _.create(null), + configs: _.create(null), + defaulted: _.create(null), + nargs: _.create(null), + coercions: _.create(null) } var negative = /^-[0-9]+(\.[0-9]+)?/ @@ -67,11 +68,11 @@ function parse (args, opts) { flags.normalize[key] = true }) - Object.keys(opts.narg || {}).forEach(function (k) { + Object.keys(opts.narg || _.create(null)).forEach(function (k) { flags.nargs[k] = opts.narg[k] }) - Object.keys(opts.coerce || {}).forEach(function (k) { + Object.keys(opts.coerce || _.create(null)).forEach(function (k) { flags.coercions[k] = opts.coerce[k] }) @@ -80,7 +81,7 @@ function parse (args, opts) { flags.configs[key] = true }) } else { - Object.keys(opts.config || {}).forEach(function (k) { + Object.keys(opts.config || _.create(null)).forEach(function (k) { flags.configs[k] = opts.config[k] }) } @@ -92,7 +93,7 @@ function parse (args, opts) { // apply default values to all aliases. Object.keys(defaults).forEach(function (key) { (flags.aliases[key] || []).forEach(function (alias) { - defaults[alias] = defaults[key] + _.set(defaults, alias, defaults[key]) }) }) @@ -417,7 +418,7 @@ function parse (args, opts) { // set args from config.json file, this should be // applied last so that defaults can be applied. function setConfig (argv) { - var configLookup = {} + var configLookup = _.create(null) // expand defaults/aliases, in-case any happen to reference // the config.json file. @@ -537,7 +538,7 @@ function parse (args, opts) { if (!configuration['dot-notation']) keys = [keys.join('.')] keys.slice(0, -1).forEach(function (key) { - o = (o[key] || {}) + o = (o[key] || _.create(null)) }) var key = keys[keys.length - 1] @@ -547,44 +548,44 @@ function parse (args, opts) { } function setKey (obj, keys, value) { - var o = obj - - if (!configuration['dot-notation']) keys = [keys.join('.')] + if (!configuration['dot-notation']) keys = keys.join('/') - keys.slice(0, -1).forEach(function (key) { - if (o[key] === undefined) o[key] = {} - o = o[key] - }) + _.update(obj, keys, updater) - var key = keys[keys.length - 1] + if (!configuration['dot-notation']) { + obj[keys.replace('/', '.')] = obj[keys] + delete obj[keys] + } - var isTypeArray = checkAllAliases(key, flags.arrays) - var isValueArray = Array.isArray(value) - var duplicate = configuration['duplicate-arguments-array'] - - if (value === increment) { - o[key] = increment(o[key]) - } else if (Array.isArray(o[key])) { - if (duplicate && isTypeArray && isValueArray) { - o[key] = configuration['flatten-duplicate-arrays'] ? o[key].concat(value) : [o[key]].concat([value]) - } else if (!duplicate && Boolean(isTypeArray) === Boolean(isValueArray)) { - o[key] = value + function updater (existing) { + var isTypeArray = checkAllAliases(key, flags.arrays) + var isValueArray = Array.isArray(value) + var duplicate = configuration['duplicate-arguments-array'] + + if (value === increment) { + return increment(existing) + } else if (Array.isArray(existing)) { + if (duplicate && isTypeArray && isValueArray) { + return configuration['flatten-duplicate-arrays'] ? existing.concat(value) : [existing].concat([value]) + } else if (!duplicate && Boolean(isTypeArray) === Boolean(isValueArray)) { + return value + } else { + return existing.concat([value]) + } + } else if (existing === undefined && isTypeArray) { + return isValueArray ? value : [value] + } else if (duplicate && !(existing === undefined || checkAllAliases(key, flags.bools) || checkAllAliases(keys.join('.'), flags.bools) || checkAllAliases(key, flags.counts))) { + return [ existing, value ] } else { - o[key] = o[key].concat([value]) + return value } - } else if (o[key] === undefined && isTypeArray) { - o[key] = isValueArray ? value : [value] - } else if (duplicate && !(o[key] === undefined || checkAllAliases(key, flags.bools) || checkAllAliases(keys.join('.'), flags.bools) || checkAllAliases(key, flags.counts))) { - o[key] = [ o[key], value ] - } else { - o[key] = value } } // extend the aliases list with inferred aliases. function extendAliases () { Array.prototype.slice.call(arguments).forEach(function (obj) { - Object.keys(obj || {}).forEach(function (key) { + Object.keys(obj || _.create(null)).forEach(function (key) { // short-circuit if we've already added a key // to the aliases array, for example it might // exist in both 'opts.default' and 'opts.key'. @@ -681,7 +682,7 @@ function parse (args, opts) { function combineAliases (aliases) { var aliasArrays = [] var change = true - var combined = {} + var combined = _.create(null) // turn alias lookup hash {key: ['alias1', 'alias2']} into // a simple array ['key', 'alias1', 'alias2'] @@ -723,20 +724,6 @@ function combineAliases (aliases) { return combined } -function assign (defaults, configuration) { - var o = {} - configuration = configuration || {} - - Object.keys(defaults).forEach(function (k) { - o[k] = defaults[k] - }) - Object.keys(configuration).forEach(function (k) { - o[k] = configuration[k] - }) - - return o -} - // this function should only be called when a count is given as an arg // it is NOT called to set a default value // thus we can start the count at 1 instead of 0 diff --git a/test/yargs-parser.js b/test/yargs-parser.js index 4826b919..53abad98 100644 --- a/test/yargs-parser.js +++ b/test/yargs-parser.js @@ -2397,7 +2397,7 @@ describe('yargs-parser', function () { }) // Patching for https://snyk.io/vuln/SNYK-JS-YARGSPARSER-560381 - it('should not pollute the prototype', function () { + it.skip('should not pollute the prototype', function () { parser(['-f.__proto__.foo', '99', '-x.y.__proto__.bar', '100', '--__proto__', '200']) Object.keys({}.__proto__).length.should.equal(0) // eslint-disable-line expect({}.foo).to.equal(undefined) From 50fc7597888bb93d6d23630cf47e65f235018743 Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Thu, 21 May 2020 01:44:41 -0700 Subject: [PATCH 3/3] Tests passing --- index.js | 109 +++++++++++++++++++++++-------------------- package.json | 3 +- test/yargs-parser.js | 2 +- 3 files changed, 61 insertions(+), 53 deletions(-) diff --git a/index.js b/index.js index 00374e15..25110c56 100644 --- a/index.js +++ b/index.js @@ -2,16 +2,16 @@ var camelCase = require('camelcase') var path = require('path') var tokenizeArgString = require('./lib/tokenize-arg-string') var util = require('util') -var _ = require('lodash') +var assign = require('object.assign') function parse (args, opts) { - if (!opts) opts = _.create(null) + if (!opts) opts = Object.create(null) // allow a string argument to be passed in rather // than an argv array. args = tokenizeArgString(args) // aliases might have transitive relationships, normalize this. - var aliases = combineAliases(opts.alias || _.create(null)) - var configuration = _.assign({ + var aliases = combineAliases(opts.alias || Object.create(null)) + var configuration = assign({ 'short-option-groups': true, 'camel-case-expansion': true, 'dot-notation': true, @@ -20,27 +20,27 @@ function parse (args, opts) { 'duplicate-arguments-array': true, 'flatten-duplicate-arrays': true }, opts.configuration) - var defaults = opts.default || _.create(null) + var defaults = opts.default || Object.create(null) var configObjects = opts.configObjects || [] var envPrefix = opts.envPrefix - var newAliases = _.create(null) + var newAliases = Object.create(null) // allow a i18n handler to be passed in, default to a fake one (util.format). var __ = opts.__ || function (str) { return util.format.apply(util, Array.prototype.slice.call(arguments)) } var error = null var flags = { - aliases: _.create(null), - arrays: _.create(null), - bools: _.create(null), - strings: _.create(null), - numbers: _.create(null), - counts: _.create(null), - normalize: _.create(null), - configs: _.create(null), - defaulted: _.create(null), - nargs: _.create(null), - coercions: _.create(null) + aliases: Object.create(null), + arrays: Object.create(null), + bools: Object.create(null), + strings: Object.create(null), + numbers: Object.create(null), + counts: Object.create(null), + normalize: Object.create(null), + configs: Object.create(null), + defaulted: Object.create(null), + nargs: Object.create(null), + coercions: Object.create(null) } var negative = /^-[0-9]+(\.[0-9]+)?/ @@ -68,11 +68,11 @@ function parse (args, opts) { flags.normalize[key] = true }) - Object.keys(opts.narg || _.create(null)).forEach(function (k) { + Object.keys(opts.narg || Object.create(null)).forEach(function (k) { flags.nargs[k] = opts.narg[k] }) - Object.keys(opts.coerce || _.create(null)).forEach(function (k) { + Object.keys(opts.coerce || Object.create(null)).forEach(function (k) { flags.coercions[k] = opts.coerce[k] }) @@ -81,7 +81,7 @@ function parse (args, opts) { flags.configs[key] = true }) } else { - Object.keys(opts.config || _.create(null)).forEach(function (k) { + Object.keys(opts.config || Object.create(null)).forEach(function (k) { flags.configs[k] = opts.config[k] }) } @@ -93,7 +93,7 @@ function parse (args, opts) { // apply default values to all aliases. Object.keys(defaults).forEach(function (key) { (flags.aliases[key] || []).forEach(function (alias) { - _.set(defaults, alias, defaults[key]) + defaults[alias] = defaults[key] }) }) @@ -418,7 +418,7 @@ function parse (args, opts) { // set args from config.json file, this should be // applied last so that defaults can be applied. function setConfig (argv) { - var configLookup = _.create(null) + var configLookup = Object.create(null) // expand defaults/aliases, in-case any happen to reference // the config.json file. @@ -538,7 +538,7 @@ function parse (args, opts) { if (!configuration['dot-notation']) keys = [keys.join('.')] keys.slice(0, -1).forEach(function (key) { - o = (o[key] || _.create(null)) + o = (o[key] || Object.create(null)) }) var key = keys[keys.length - 1] @@ -548,44 +548,46 @@ function parse (args, opts) { } function setKey (obj, keys, value) { - if (!configuration['dot-notation']) keys = keys.join('/') + var o = obj - _.update(obj, keys, updater) + if (!configuration['dot-notation']) keys = [keys.join('.')] - if (!configuration['dot-notation']) { - obj[keys.replace('/', '.')] = obj[keys] - delete obj[keys] - } + keys = keys.map(sanitizeKey) - function updater (existing) { - var isTypeArray = checkAllAliases(key, flags.arrays) - var isValueArray = Array.isArray(value) - var duplicate = configuration['duplicate-arguments-array'] - - if (value === increment) { - return increment(existing) - } else if (Array.isArray(existing)) { - if (duplicate && isTypeArray && isValueArray) { - return configuration['flatten-duplicate-arrays'] ? existing.concat(value) : [existing].concat([value]) - } else if (!duplicate && Boolean(isTypeArray) === Boolean(isValueArray)) { - return value - } else { - return existing.concat([value]) - } - } else if (existing === undefined && isTypeArray) { - return isValueArray ? value : [value] - } else if (duplicate && !(existing === undefined || checkAllAliases(key, flags.bools) || checkAllAliases(keys.join('.'), flags.bools) || checkAllAliases(key, flags.counts))) { - return [ existing, value ] + keys.slice(0, -1).forEach(function (key) { + if (o[key] === undefined) o[key] = Object.create(null) + o = o[key] + }) + + var key = keys[keys.length - 1] + + var isTypeArray = checkAllAliases(key, flags.arrays) + var isValueArray = Array.isArray(value) + var duplicate = configuration['duplicate-arguments-array'] + + if (value === increment) { + o[key] = increment(o[key]) + } else if (Array.isArray(o[key])) { + if (duplicate && isTypeArray && isValueArray) { + o[key] = configuration['flatten-duplicate-arrays'] ? o[key].concat(value) : [o[key]].concat([value]) + } else if (!duplicate && Boolean(isTypeArray) === Boolean(isValueArray)) { + o[key] = value } else { - return value + o[key] = o[key].concat([value]) } + } else if (o[key] === undefined && isTypeArray) { + o[key] = isValueArray ? value : [value] + } else if (duplicate && !(o[key] === undefined || checkAllAliases(key, flags.bools) || checkAllAliases(keys.join('.'), flags.bools) || checkAllAliases(key, flags.counts))) { + o[key] = [ o[key], value ] + } else { + o[key] = value } } // extend the aliases list with inferred aliases. function extendAliases () { Array.prototype.slice.call(arguments).forEach(function (obj) { - Object.keys(obj || _.create(null)).forEach(function (key) { + Object.keys(obj || Object.create(null)).forEach(function (key) { // short-circuit if we've already added a key // to the aliases array, for example it might // exist in both 'opts.default' and 'opts.key'. @@ -682,7 +684,7 @@ function parse (args, opts) { function combineAliases (aliases) { var aliasArrays = [] var change = true - var combined = _.create(null) + var combined = Object.create(null) // turn alias lookup hash {key: ['alias1', 'alias2']} into // a simple array ['key', 'alias1', 'alias2'] @@ -731,6 +733,11 @@ function increment (orig) { return orig !== undefined ? orig + 1 : 1 } +function sanitizeKey (key) { + if (key === '__proto__') return '___proto___' + return key +} + function Parser (args, opts) { var result = parse(args.slice(), opts) diff --git a/package.json b/package.json index d025a7ef..96af5496 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,8 @@ "standard-version": "^4.0.0" }, "dependencies": { - "camelcase": "^3.0.0" + "camelcase": "^3.0.0", + "object.assign": "^4.1.0" }, "files": [ "lib", diff --git a/test/yargs-parser.js b/test/yargs-parser.js index 53abad98..4826b919 100644 --- a/test/yargs-parser.js +++ b/test/yargs-parser.js @@ -2397,7 +2397,7 @@ describe('yargs-parser', function () { }) // Patching for https://snyk.io/vuln/SNYK-JS-YARGSPARSER-560381 - it.skip('should not pollute the prototype', function () { + it('should not pollute the prototype', function () { parser(['-f.__proto__.foo', '99', '-x.y.__proto__.bar', '100', '--__proto__', '200']) Object.keys({}.__proto__).length.should.equal(0) // eslint-disable-line expect({}.foo).to.equal(undefined)