jonschlinkert / kind-of

Get the native JavaScript type of a value, fast. Used by superstruct, micromatch and many others!
https://github.com/jonschlnkert
MIT License
347 stars 38 forks source link

Security vulnerability backport to 3.x line. #33

Closed mcollina closed 4 years ago

mcollina commented 4 years ago

Chokidar@2 depends on this module in version 3.x. It would be great to release the security fix to this line, so that Chokidar@2 is not vulnerable anymore.

I think the security fix is https://github.com/jonschlinkert/kind-of/commit/975c13a7cfaf25d811475823824af3a9c04b0ba8. Is this the only thing?

I'm happy to do the PR to backport the change if you cut me a 3.x branch

doowb commented 4 years ago

Hi @mcollina, thank you for the offer, but the constructor check wasn't added until 6.x so there shouldn't be any issues now.

Do you have any ideas on how to update the vulnerability databases and tools if they're reporting other versions being affected?

mcollina commented 4 years ago

@lirantal can you help?

lirantal commented 4 years ago

Yes let me get up to speed on things here quick

lirantal commented 4 years ago

I do see references that access the constructor key in prior to 6.x versions like this one:

image

From: https://www.unpkg.com/browse/kind-of@5.1.0/index.js

However, it seems that this is safe due to the prior check in line 31 for type being a function. I see that this has been introduced since version 5.x. 4.x and prior indeed has no constructor key access mentioned from what I can tell.

doowb commented 4 years ago

I should have been more specific instead of just saying constructor. The fix mentioned is for https://nvd.nist.gov/vuln/detail/CVE-2019-20149. The code in question wasn't introduced until 6.0.

The issue that I see is that the databases and tools must be flagging other versions that are unaffected, but as a maintainer of this project, I don't know what the process is to request an update.

lirantal commented 4 years ago

Oh no worries, we're getting that clarified from the Snyk database side of things to update affected versions, I just wanted to confirm that indeed even though the constructor key is being referred to in 5.x, it is really only affected in 6.x

Thanks for confirming. We'll quickly update the database on our side and keep you informed. Apologies for the confusion and appreciate your time on this 💜

lirantal commented 4 years ago

We're going to flag vulnerable versions as >= 6.0.0 and < 6.0.3. It's not Snyk who issued the CVE request for NVD so not sure why it was flagged as only 6.0.2 vulnerable but we'll do our best to reach out and ask them to correct.

Any scans or database checks based on Snyk's tooling will be fixed for the correct version. If anything comes up on this or otherwise I'm always happy to help, just tag me.

doowb commented 4 years ago

Thank you @lirantal (now I know the process... ping you 😉)

@mcollina will you confirm this is fine in chokidar@2? (and/or the project linked in your related issue)

mcollina commented 4 years ago

That dependency tree uses kind-of@3.2.2 and kind-of@5.1.0, so I think we should be good.

lirantal commented 4 years ago

Thanks Matteo for quickly flagging this ❤️

mcollina commented 4 years ago

Let me know once the db is updated.

mcollina commented 4 years ago

Actually, it's already fixed.

lirantal commented 4 years ago

Yep :-)

adelyafatykhova commented 4 years ago

@doowb (or anyone else), this issue still appears when using clone-deep; I opened a PR to update the version of kind-of being used to the latest: https://github.com/jonschlinkert/clone-deep/pull/18

Could someone who is able to take care of it and release a new version?

lirantal commented 4 years ago

Which tool are you using that flags this vulnerability?

julienw commented 4 years ago

github says kind-of < 6.0.3. However when upgrading version 6.0.2 to 6.0.3, but keeping the versions below 6, the security issue disappear. So I believe github rightly reported the issue because I had some dependencies to 6.0.2, but the title was wrong.