Critical security vulnerability

[Retr02332]

Hello !!

Do you have an email where I can send a security vulnerability?

Regards.

[mattphillips]

Hi @Retr02332 you can DM me on Twitter

[chrisbujok]

Is the vulnerability still valid? Would be great to know if there's anything to pay attention to.

[Retr02332]

I spoke with Matt via DM and a validation of the user input was reached. Specifically, on the keyword "__proto__".

[diracdeltas]

is there a fix for this yet? it's causing dependabot alerts for a number of repos

[mattphillips]

Hey I'd like some feedback on this issue, I've written tests based on the vulnerability shared by @Retr02332 to me and outlined in this article: https://learn.snyk.io/lessons/prototype-pollution/javascript.

You can see the tests here: https://github.com/mattphillips/deep-object-diff/blob/main/test/pollution.test.js

From my understanding of this vulnerability a bad actor would be able to globally pollute all JS objects prototype. I have not been able to replicate this in my investigations into this issue and in the tests above.

The only "pollution" I'm able to achieve is in the third test where in userland I am unsafely trusting JSON.parse and again in userland am mutating the original object that is being diffed. The "pollution" achieved is only applied to the original object (because of the mutation not the diff algorithm) and not to the global prototype as outlined in the above article. I'm not even sure we should call this example "prototype pollution" to be honest (happy to be proven wrong on this).

I'm really not sure there is a vulnerability (am happy to be proven wrong and to fix it) so could someone please show me a global prototype pollution with this library? Otherwise I feel that the vulnerability listed in dependabot etc should be removed.

Also if I am correct and this is not an issue, @Retr02332 how do I close all of these vulnerability tickets that have been created on various sites?

[Retr02332]

Hi @mattphillips !!

I do not poison the prototype of all objects, only of the object that is used for the unsafe operation. However, this is still a prototype pollution.

I have achieved several bypasses to authentication thanks to this problem. However, if you do not consider this to be a security flaw, let me know, and I will be happy to close this issue.

• https://fluidattacks.com/advisories/heldens/

[mattphillips]

@Retr02332 I've just opened a PR #87 that remedies polluting the prototype of the returned object. Could you please take a look to see if it does fully resolve the issue you have identified?

I'm still a little bit dubious to call this a security flaw but it's a fairly trivial fix so worth doing. The only question I have now is does this constitute a breaking change in this libraries behaviour and what version should this be released on?

I suppose it is not a breaking change as this issue was introduced in 4c855be and released in v1.1.6 - so releasing this fix as a patch is correct as prior to v1.1.6 this issue was not present.

[mattphillips]

Fix has been released in https://github.com/mattphillips/deep-object-diff/releases/tag/v1.19

@Retr02332 how do we go about updating dependabot etc to reflect that the fix has been released?

[diracdeltas]

I'm not sure if there is a more "official" way to tell NPM about the fix, but I submitted a request to Github here: github/advisory-database#819

[Retr02332]

@mattphillips Do you mean the advisories?

I will update the Advisory on Tuesday to indicate the presence of a patch. It is only a matter of time before this starts to be replicated on the other sites that track the case.

[anogr]

Fix has been released in https://github.com/mattphillips/deep-object-diff/releases/tag/v1.19

@Retr02332 how do we go about updating dependabot etc to reflect that the fix has been released?

@mattphillips We're having false warnings. It appears to be a typo, the fix is in version 1.1.9, not in 1.1.19.

[mattphillips]

@anogr yes you're correct that is a typo in the GitHub tag of the release. The fix is in v1.1.9 see the release on npm here https://www.npmjs.com/package/deep-object-diff/v/1.1.9

[anogr]

@mattphillips We're having false warning about version 1.1.19. But I see that there are open PRs that fix that.

[mattphillips]

@anogr I have no idea - I don't know where pnpm audit get the versions from.

@Retr02332 any ideas?

[RahulPol]

@mattphillips I think its because of the wrong tag

[mattphillips]

@anogr @RahulPol I've just updated the Github Advisory of the issue here: github/advisory-database#834

It appears that it was configured with the typo of v1.1.19 so hopefully when that is merged this secondary issue should be resolved.