security: switch to using Map during parsing

[bcoe]

Once we merge into the Node.js codebase, getting security patches out the door becomes more painful.

I've had to fix several prototype related bugs/security notifications over the years by virtue of using a POJO when parsing. Here's an example as to how this can cause weird behavior:

> const obj = {__proto__: {a: 99}}
undefined
> obj.a
99

I think it would be worth preemptively moving our implementation to using a Map, and then converting the Map into an object as a final step.

[shadowspawn]

The issues I looked at that were reported against Yargs and Minimist were due to supporting structured options, where --foo.bar is parsed into { foo: { bar: true } }. We are doing single level assignment in parseArgs, although we do also have arrays.

I'm not against using SafeMap, but I don't think we are exposed to the same risks.

(Previous PR using SafeMap without tests: #65)

[bcoe]

The issues I looked at that were reported against Yargs and Minimist were due to supporting structured options, where --foo.bar

I agree, potentially I'm being too paranoid. I wasn't sure if:

const obj = {__proto__: 99}

Is cause for a alarm? as far as I can tell the key just get dropped.

[shadowspawn]

There is now a test in StoreOptionValue to return early if the key is __proto__ (thanks to @bcoe).

See also #108 for low tech map, the null prototype object.

I suggest SafeMap is more churn than useful and close this issue?

[bcoe]

👍 let's close this issue and concentrate on #104