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.

Sign up for GitHub

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
Merged

fix: resist pollution #106

Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b946c3a
Proof of concept of pollution protection
shadowspawn Apr 13, 2022
fc9ff43
Update index.js
shadowspawn Apr 14, 2022
6d2351a
Tighten up index.js code against prototype pollution
shadowspawn Apr 14, 2022
9c7d5d4
Move helpers to utils
shadowspawn Apr 14, 2022
dab667b
Merge remote-tracking branch 'upstream/main' into feature/pollution-p…
shadowspawn Apr 14, 2022
54e221a
Make utils more robust to pollution
shadowspawn Apr 14, 2022
3ce5a47
Likely temporary ObjectDefineProperty paranoia
shadowspawn Apr 14, 2022
0daf602
Merge branch 'main' into feature/pollution-proof
shadowspawn Apr 15, 2022
ec4a81d
Add JSDoc for Check and Store
shadowspawn Apr 15, 2022
501c631
Add pollution tests
shadowspawn Apr 15, 2022
c684916
Combined test files
shadowspawn Apr 15, 2022
99768b5
Make set/restore prototype do more faithful restore
shadowspawn Apr 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 61 additions & 63 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,61 +74,66 @@ 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] : {};
// ToDo: move to utils.js
function objectGetOwn(obj, prop) {
if (ObjectHasOwn(obj, prop))
return obj[prop];
}

if (strict) {
if (!hasOptionConfig) {
throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOption == null ? `--${longOption}` : `-${shortOption}`);
}
function optionsGetOwn(options, longOption, prop) {
if (ObjectHasOwn(options, longOption))
return objectGetOwn(options[longOption], prop);
}

const shortOptionErr = ObjectHasOwn(optionConfig, 'short') ? `-${optionConfig.short}, ` : '';
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) {
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,
// 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 {
values[longOption] = [newValue];
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
result.values[longOption] = newValue;
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 +142,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 +189,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 +205,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 +225,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 +249,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