Minimist throws on options like '--toString' '--constructor', '--_proto_'

[user58823]

With the following minimal script, similar to the example from the README.md:

#!/usr/bin/env node
console.dir(require('minimist')(process.argv.slice(2)));

Running the file with options like --toString, --hasOwnProperty, --constructor, --__proto__ etc. causes minimist (version 1.2.8) to throw this error:

[...]/node_modules/minimist/index.js:127
        (aliases[key] || []).forEach(function (x) {
                             ^
TypeError: (aliases[key] || []).forEach is not a function
    at setArg ([...]/node_modules/minimist/index.js:127:24)
    at module.exports ([...]/node_modules/minimist/index.js:178:5)
    at Object.<anonymous> ([...]/minimist-test.js:2:32)
    [...]
Node.js v18.17.1

From a quick glance at the source, changing the line var aliases = {}; to var aliases = Object.create(null); makes it no longer throw, but still gives weird results:

$ ./minimist-test.js --expected test
{ _: [], expected: 'test' }
$ ./minimist-test.js --hasOwnProperty test
{ _: [ 'test' ], hasOwnProperty: '' }

I assume there are other objects that should have a null prototype somewhere (which would probably also help with the "prototype pollution" problems you seem to be having).

[shadowspawn]

Minimist is using a plain object to store the parsing results, with a resulting clash if you use options that match standard properties.

Is this something that affected something you wanted to do, or were you just interested in what happens?

(Using a null prototype is slowly becoming more mainstream but wasn't the approach used at the time the "prototype pollution" problems were identified, and a lower impact approach was taken to fix those.)

[ljharb]

We’d use { __proto__: null } instead of Object.create; it’s faster and more robust. I’m fine making that change though; it’s been a bad practice to call object prototype methods directly on objects for decades now anyways.