fix: 🐛 fix prototype pollution

[AdamGold]

No description provided.

[keithamus]

Hey @AdamGold thanks for making this PR!

I remain unconvinced this is really a security issue. pathval is basically a library implementing = and as such anything you can do with = you can do with pathval.

Additionally, if we were to consider this a security issue, where does the line get drawn? constructor,prototype, & __proto__ are just psuedo protocols like .then or .toString, do we also need to guard against those? Should we guard against methods a user might call, such as .map? It is an intrinsic part of the language that you need to take care that values you pass to functions can be mutated.

See also #57

[AdamGold]

Hey @keithamus!

I definitely agree that this is a controversial vulnerability. Full disclosure - I work for Snyk, and we have dealt with quite a few of these. I understand your point, I would just like to clarify that the examples you've given - .map or .toString affect the current object, as opposed to __proto__, prototype which could modify global behaviour and cause more severe issues.

I think that this is the main reason for which this PR could be useful, and the line should be drawn there. With that being said, I am fine with you closing this PR if you still think we should not implement these checks.

BTW I can not see the README warning anymore, has it been removed? (edit: sorry did not notice it was not merged!)

[keithamus]

The README warning was not merged, as no one has approved it yet (it's sitting here: #57).

I'd like to hear from other members of @chaijs/core about this - perhaps @meeber or @lucasfcosta?

[ahippler]

Any news on this?

[donal-tobin-sap]

CVE: https://nvd.nist.gov/vuln/detail/CVE-2020-7751

[stfsy]

@meeber or @lucasfcosta could you give an update on this? looks like an improvement to me. so why not merge it?

[deleonio]

Thanks @AdamGold - this PR fix the vulnerabilty + tests + dep updates | #60

[deleonio]

Please use PR #60

Because it contains this fix code in addition of tests and needed update tool config for git workflow

[stfsy]

@keithamus as @kckst8 approved this PR. are we ready to merge, release etc..? 🙂

[yahanvesh]

If someone can review and fix, it will be great!

[keithamus]

Released as v1.1.1

[JBallin]

For reference - the fix was merged in #60