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: resist pollution #106

Merged
merged 12 commits into from
Apr 15, 2022
150 changes: 84 additions & 66 deletions index.js
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
});
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved
};

// 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)) {
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
ArrayPrototypeFind,
ObjectEntries,
ObjectPrototypeHasOwnProperty: ObjectHasOwn,
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved
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);
}
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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
};