Skip to content

Commit

Permalink
fix: resist pollution (#106)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shadowspawn
shadowspawn committed Apr 15, 2022
1 parent e5d1f4b commit ecf2dec
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 70 deletions.
150 changes: 84 additions & 66 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ const {
ArrayPrototypeShift,
ArrayPrototypeSlice,
ArrayPrototypePush,
ObjectPrototypeHasOwnProperty: ObjectHasOwn,
ObjectDefineProperty,
ObjectEntries,
ObjectPrototypeHasOwnProperty: ObjectHasOwn,
StringPrototypeCharAt,
StringPrototypeIncludes,
StringPrototypeIndexOf,
Expand All @@ -29,7 +30,9 @@ const {
isLongOptionAndValue,
isOptionValue,
isShortOptionAndValue,
isShortOptionGroup
isShortOptionGroup,
objectGetOwn,
optionsGetOwn
} = require('./utils');

const {
Expand Down Expand Up @@ -74,61 +77,83 @@ function getMainArgs() {
return ArrayPrototypeSlice(process.argv, 2);
}

const protoKey = '__proto__';

function storeOption({
strict,
options,
result,
longOption,
shortOption,
optionValue,
}) {
const hasOptionConfig = ObjectHasOwn(options, longOption);
const optionConfig = hasOptionConfig ? options[longOption] : {};

if (strict) {
if (!hasOptionConfig) {
throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOption == null ? `--${longOption}` : `-${shortOption}`);
}

const shortOptionErr = ObjectHasOwn(optionConfig, 'short') ? `-${optionConfig.short}, ` : '';
/**
* In strict mode, throw for usage errors.
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {string|undefined} optionValue - value from user args
* @param {Object} options - option configs, from parseArgs({ options })
* @param {string} shortOrLong - option used, with dashes e.g. `-l` or `--long`
* @param {boolean} strict - show errors, from parseArgs({ strict })
*/
function checkOptionUsage(longOption, optionValue, options,
shortOrLong, strict) {
// Strict and options are used from local context.
if (!strict) return;

if (options[longOption].type === 'string' && optionValue == null) {
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption} <value>' argument missing`);
}
if (!ObjectHasOwn(options, longOption)) {
throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOrLong);
}

if (options[longOption].type === 'boolean' && optionValue != null) {
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption}' does not take an argument`);
}
const short = optionsGetOwn(options, longOption, 'short');
const shortAndLong = short ? `-${short}, --${longOption}` : `--${longOption}`;
const type = optionsGetOwn(options, longOption, 'type');
if (type === 'string' && typeof optionValue !== 'string') {
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong} <value>' argument missing`);
}
// (Idiomatic test for undefined||null, expecting undefined.)
if (type === 'boolean' && optionValue != null) {
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong}' does not take an argument`);
}
}

if (longOption === protoKey) {
/**
* Store the option value in `values`.
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {string|undefined} optionValue - value from user args
* @param {Object} options - option configs, from parseArgs({ options })
* @param {Object} values - option values returned in `values` by parseArgs
*/
function storeOption(longOption, optionValue, options, values) {
if (longOption === '__proto__') {
return;
}

// Values
const usedAsFlag = optionValue === undefined;
const newValue = usedAsFlag ? true : optionValue;
if (optionConfig.multiple) {
// Always store value in array, including for flags.
// result.values[longOption] starts out not present,
// Can be removed when value has a null prototype
const safeAssignProperty = (obj, prop, value) => {
ObjectDefineProperty(obj, prop, {
value,
writable: true,
enumerable: true,
configurable: true
});
};

// We store based on the option value rather than option type,
// preserving the users intent for author to deal with.
const newValue = optionValue ?? true;
if (optionsGetOwn(options, longOption, 'multiple')) {
// Always store value in array, including for boolean.
// values[longOption] starts out not present,
// first value is added as new array [newValue],
// subsequent values are pushed to existing array.
if (result.values[longOption] !== undefined)
ArrayPrototypePush(result.values[longOption], newValue);
else
result.values[longOption] = [newValue];
if (ObjectHasOwn(values, longOption)) {
ArrayPrototypePush(values[longOption], newValue);
} else {
safeAssignProperty(values, longOption, [newValue]);
}
} else {
result.values[longOption] = newValue;
safeAssignProperty(values, longOption, newValue);
}
}

const parseArgs = ({
args = getMainArgs(),
strict = true,
options = {}
} = {}) => {
const parseArgs = (config = { __proto__: null }) => {
const args = objectGetOwn(config, 'args') ?? getMainArgs();
const strict = objectGetOwn(config, 'strict') ?? true;
const options = objectGetOwn(config, 'options') ?? { __proto__: null };

// Validate input configuration.
validateArray(args, 'args');
validateBoolean(strict, 'strict');
validateObject(options, 'options');
Expand All @@ -137,7 +162,8 @@ const parseArgs = ({
({ 0: longOption, 1: optionConfig }) => {
validateObject(optionConfig, `options.${longOption}`);

validateUnion(optionConfig.type, `options.${longOption}.type`, ['string', 'boolean']);
// type is required
validateUnion(objectGetOwn(optionConfig, 'type'), `options.${longOption}.type`, ['string', 'boolean']);

if (ObjectHasOwn(optionConfig, 'short')) {
const shortOption = optionConfig.short;
Expand Down Expand Up @@ -183,18 +209,13 @@ const parseArgs = ({
const shortOption = StringPrototypeCharAt(arg, 1);
const longOption = findLongOptionForShort(shortOption, options);
let optionValue;
if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) {
if (optionsGetOwn(options, longOption, 'type') === 'string' &&
isOptionValue(nextArg)) {
// e.g. '-f', 'bar'
optionValue = ArrayPrototypeShift(remainingArgs);
}
storeOption({
strict,
options,
result,
longOption,
shortOption,
optionValue,
});
checkOptionUsage(longOption, optionValue, options, arg, strict);
storeOption(longOption, optionValue, options, result.values);
continue;
}

Expand All @@ -204,7 +225,7 @@ const parseArgs = ({
for (let index = 1; index < arg.length; index++) {
const shortOption = StringPrototypeCharAt(arg, index);
const longOption = findLongOptionForShort(shortOption, options);
if (options[longOption]?.type !== 'string' ||
if (optionsGetOwn(options, longOption, 'type') !== 'string' ||
index === arg.length - 1) {
// Boolean option, or last short in group. Well formed.
ArrayPrototypePush(expanded, `-${shortOption}`);
Expand All @@ -224,26 +245,22 @@ const parseArgs = ({
const shortOption = StringPrototypeCharAt(arg, 1);
const longOption = findLongOptionForShort(shortOption, options);
const optionValue = StringPrototypeSlice(arg, 2);
storeOption({
strict,
options,
result,
longOption,
shortOption,
optionValue,
});
checkOptionUsage(longOption, optionValue, options, `-${shortOption}`, strict);
storeOption(longOption, optionValue, options, result.values);
continue;
}

if (isLoneLongOption(arg)) {
// e.g. '--foo'
const longOption = StringPrototypeSlice(arg, 2);
let optionValue;
if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) {
if (optionsGetOwn(options, longOption, 'type') === 'string' &&
isOptionValue(nextArg)) {
// e.g. '--foo', 'bar'
optionValue = ArrayPrototypeShift(remainingArgs);
}
storeOption({ strict, options, result, longOption, optionValue });
checkOptionUsage(longOption, optionValue, options, arg, strict);
storeOption(longOption, optionValue, options, result.values);
continue;
}

Expand All @@ -252,7 +269,8 @@ const parseArgs = ({
const index = StringPrototypeIndexOf(arg, '=');
const longOption = StringPrototypeSlice(arg, 2, index);
const optionValue = StringPrototypeSlice(arg, index + 1);
storeOption({ strict, options, result, longOption, optionValue });
checkOptionUsage(longOption, optionValue, options, `--${longOption}`, strict);
storeOption(longOption, optionValue, options, result.values);
continue;
}

Expand Down
72 changes: 72 additions & 0 deletions test/prototype-pollution.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@
const test = require('tape');
const { parseArgs } = require('../index.js');

// These tests are not synced upstream with node, in case of possible side-effects.
// See index.js for tests shared with upstream.

function setObjectPrototype(prop, value) {
const oldDescriptor = Object.getOwnPropertyDescriptor(Object.prototype, prop);
Object.prototype[prop] = value;
return oldDescriptor;
}

function restoreObjectPrototype(prop, oldDescriptor) {
if (oldDescriptor == null) {
delete Object.prototype[prop];
} else {
Object.defineProperty(Object.prototype, prop, oldDescriptor);
}
}

test('should not allow __proto__ key to be set on object', (t) => {
const passedArgs = ['--__proto__=hello'];
const expected = { values: {}, positionals: [] };
Expand All @@ -13,3 +30,58 @@ test('should not allow __proto__ key to be set on object', (t) => {
t.deepEqual(result, expected);
t.end();
});

test('when prototype has multiple then ignored', (t) => {
const args = ['--foo', '1', '--foo', '2'];
const options = { foo: { type: 'string' } };
const expectedResult = { values: { foo: '2' }, positionals: [] };

const holdDescriptor = setObjectPrototype('multiple', true);
const result = parseArgs({ args, options });
restoreObjectPrototype('multiple', holdDescriptor);
t.deepEqual(result, expectedResult);
t.end();
});

test('when prototype has type then ignored', (t) => {
const args = ['--foo', '1'];
const options = { foo: { } };

const holdDescriptor = setObjectPrototype('type', 'string');
t.throws(() => {
parseArgs({ args, options });
});
restoreObjectPrototype('type', holdDescriptor);
t.end();
});

test('when prototype has short then ignored', (t) => {
const args = ['-f', '1'];
const options = { foo: { type: 'string' } };

const holdDescriptor = setObjectPrototype('short', 'f');
t.throws(() => {
parseArgs({ args, options });
});
restoreObjectPrototype('short', holdDescriptor);
t.end();
});

test('when prototype has strict then ignored', (t) => {
const args = ['-f'];

const holdDescriptor = setObjectPrototype('strict', false);
t.throws(() => {
parseArgs({ args });
});
restoreObjectPrototype('strict', holdDescriptor);
t.end();
});

test('when prototype has args then ignored', (t) => {
const holdDescriptor = setObjectPrototype('args', ['--foo']);
const result = parseArgs({ strict: false });
restoreObjectPrototype('args', holdDescriptor);
t.false(result.values.foo);
t.end();
});
27 changes: 23 additions & 4 deletions utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
ArrayPrototypeFind,
ObjectEntries,
ObjectPrototypeHasOwnProperty: ObjectHasOwn,
StringPrototypeCharAt,
StringPrototypeIncludes,
StringPrototypeSlice,
Expand All @@ -20,6 +21,22 @@ const {
//
// These routines are for internal use, not for export to client.

/**
* Return the named property, but only if it is an own property.
*/
function objectGetOwn(obj, prop) {
if (ObjectHasOwn(obj, prop))
return obj[prop];
}

/**
* Return the named options property, but only if it is an own property.
*/
function optionsGetOwn(options, longOption, prop) {
if (ObjectHasOwn(options, longOption))
return objectGetOwn(options[longOption], prop);
}

/**
* Determines if the argument may be used as an option value.
* NB: We are choosing not to accept option-ish arguments.
Expand Down Expand Up @@ -107,7 +124,7 @@ function isShortOptionGroup(arg, options) {

const firstShort = StringPrototypeCharAt(arg, 1);
const longOption = findLongOptionForShort(firstShort, options);
return options[longOption]?.type !== 'string';
return optionsGetOwn(options, longOption, 'type') !== 'string';
}

/**
Expand All @@ -128,7 +145,7 @@ function isShortOptionAndValue(arg, options) {

const shortOption = StringPrototypeCharAt(arg, 1);
const longOption = findLongOptionForShort(shortOption, options);
return options[longOption]?.type === 'string';
return optionsGetOwn(options, longOption, 'type') === 'string';
}

/**
Expand All @@ -144,7 +161,7 @@ function findLongOptionForShort(shortOption, options) {
validateObject(options, 'options');
const { 0: longOption } = ArrayPrototypeFind(
ObjectEntries(options),
({ 1: optionConfig }) => optionConfig.short === shortOption
({ 1: optionConfig }) => objectGetOwn(optionConfig, 'short') === shortOption
) || [];
return longOption || shortOption;
}
Expand All @@ -156,5 +173,7 @@ module.exports = {
isLongOptionAndValue,
isOptionValue,
isShortOptionAndValue,
isShortOptionGroup
isShortOptionGroup,
objectGetOwn,
optionsGetOwn
};

0 comments on commit ecf2dec

Please sign in to comment.