minimistjs / minimist

parse argument options
MIT License
515 stars 30 forks source link

Tests: check side-effects of pollution protection #25

Closed shadowspawn closed 1 year ago

shadowspawn commented 1 year ago

There are separate checks against the leading keys in a dotted option name than against the last key. If the checks for __proto__ or constructor are wrong in the checks of the last key then it won't lead to prototype pollution as such, but will lead to less consistent behaviour between --a.constructor.prototype.b and --a.constructor.

The pollution checks on the v0.2.x branch missed the constructor check on the last key in the dotted option name. So again this does not allow prototype pollution, but does mean there is different behaviour between the main branch and the v0.2.x branch.

This PR adds a test which fails due to the missing code, and the failing test will be fixed by #24. (Added separately so can see the test fail. Tests added high in the file to avoid a future merge conflict on mainline to slightly simplify copying up.)

If this seems worthwhile, this could be merged separately or as part of #24?

not ok 103 should be deeply equivalent
  ---
    operator: deepEqual
    expected: |-
      {}
    actual: |-
      { constructor: [ [Function: Object], 'IGNORED' ] }
    at: Test.<anonymous> (/Users/john/Documents/Sandpits/minimist/my-fork/test/proto.js:18:4)
ljharb commented 1 year ago

If not, then I think it should be part of the PR that fixes it.

shadowspawn commented 1 year ago

I'll add to other PR.