From 0598b5db9d53a2768efed3d5cfbff48cc6a4628d Mon Sep 17 00:00:00 2001 From: "bencoe@google.com" Date: Sat, 9 Apr 2022 14:59:22 +0000 Subject: [PATCH 1/2] handle prototype pollution --- index.js | 5 +++++ test/prototype-pollution.js | 15 +++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 test/prototype-pollution.js diff --git a/index.js b/index.js index 56da055..898a0db 100644 --- a/index.js +++ b/index.js @@ -66,12 +66,17 @@ function getMainArgs() { return ArrayPrototypeSlice(process.argv, 2); } +const protoKey = '__proto__'; function storeOptionValue(options, longOption, value, result) { const optionConfig = options[longOption] || {}; // Flags result.flags[longOption] = true; + if (longOption === protoKey) { + return; + } + // Values if (optionConfig.multiple) { // Always store value in array, including for flags. diff --git a/test/prototype-pollution.js b/test/prototype-pollution.js new file mode 100644 index 0000000..83b47e5 --- /dev/null +++ b/test/prototype-pollution.js @@ -0,0 +1,15 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + +test('should not allow __proto__ key to be set on object', (t) => { + const passedArgs = ['--__proto__=hello']; + const expected = { flags: {}, values: {}, positionals: [] }; + + const result = parseArgs({ args: passedArgs }); + + t.deepEqual(result, expected); + t.end(); +}); From 6c5b5df8901538bcbc383d19c9a24745c362ac54 Mon Sep 17 00:00:00 2001 From: "bencoe@google.com" Date: Sat, 9 Apr 2022 20:59:23 +0000 Subject: [PATCH 2/2] refactor: tweaks for Node.js --- errors.js | 8 ++++++++ index.js | 12 +++++++++--- test/index.js | 4 +++- utils.js | 13 +++++++++---- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/errors.js b/errors.js index 2220b66..1b9eb95 100644 --- a/errors.js +++ b/errors.js @@ -7,8 +7,16 @@ class ERR_INVALID_ARG_TYPE extends TypeError { } } +class ERR_INVALID_SHORT_OPTION extends TypeError { + constructor(longOption, shortOption) { + super(`options.${longOption}.short must be a single character, got '${shortOption}'`); + this.code = 'ERR_INVALID_SHORT_OPTION'; + } +} + module.exports = { codes: { ERR_INVALID_ARG_TYPE, + ERR_INVALID_SHORT_OPTION } }; diff --git a/index.js b/index.js index 898a0db..48bc422 100644 --- a/index.js +++ b/index.js @@ -6,7 +6,7 @@ const { ArrayPrototypeShift, ArrayPrototypeSlice, ArrayPrototypePush, - ObjectHasOwn, + ObjectPrototypeHasOwnProperty: ObjectHasOwn, ObjectEntries, StringPrototypeCharAt, StringPrototypeIncludes, @@ -32,6 +32,12 @@ const { isShortOptionGroup } = require('./utils'); +const { + codes: { + ERR_INVALID_SHORT_OPTION, + }, +} = require('./errors'); + function getMainArgs() { // This function is a placeholder for proposed process.mainArgs. // Work out where to slice process.argv for user supplied arguments. @@ -102,7 +108,7 @@ const parseArgs = ({ validateObject(options, 'options'); ArrayPrototypeForEach( ObjectEntries(options), - ([longOption, optionConfig]) => { + ({ 0: longOption, 1: optionConfig }) => { validateObject(optionConfig, `options.${longOption}`); if (ObjectHasOwn(optionConfig, 'type')) { @@ -113,7 +119,7 @@ const parseArgs = ({ const shortOption = optionConfig.short; validateString(shortOption, `options.${longOption}.short`); if (shortOption.length !== 1) { - throw new Error(`options.${longOption}.short must be a single character, got '${shortOption}'`); + throw new ERR_INVALID_SHORT_OPTION(longOption, shortOption); } } diff --git a/test/index.js b/test/index.js index a393bf8..e995d76 100644 --- a/test/index.js +++ b/test/index.js @@ -385,7 +385,9 @@ test('invalid short option length', function(t) { const passedArgs = []; const passedOptions = { foo: { short: 'fo' } }; - t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }); + t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_INVALID_SHORT_OPTION' + }); t.end(); }); diff --git a/utils.js b/utils.js index eca8711..f3cbbff 100644 --- a/utils.js +++ b/utils.js @@ -9,6 +9,10 @@ const { StringPrototypeStartsWith, } = require('./primordials'); +const { + validateObject +} = require('./validators'); + // These are internal utilities to make the parsing logic easier to read, and // add lots of detail for the curious. They are in a separate file to allow // unit testing, although that is not essential (this could be rolled into @@ -116,7 +120,8 @@ function isShortOptionGroup(arg, options) { * }) // returns true */ function isShortOptionAndValue(arg, options) { - if (!options) throw new Error('Internal error, missing options argument'); + validateObject(options, 'options'); + if (arg.length <= 2) return false; if (StringPrototypeCharAt(arg, 0) !== '-') return false; if (StringPrototypeCharAt(arg, 1) === '-') return false; @@ -136,10 +141,10 @@ function isShortOptionAndValue(arg, options) { * }) // returns 'bar' */ function findLongOptionForShort(shortOption, options) { - if (!options) throw new Error('Internal error, missing options argument'); - const [longOption] = ArrayPrototypeFind( + validateObject(options, 'options'); + const { 0: longOption } = ArrayPrototypeFind( ObjectEntries(options), - ([, optionConfig]) => optionConfig.short === shortOption + ({ 1: optionConfig }) => optionConfig.short === shortOption ) || []; return longOption || shortOption; }