Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code injection security vulnerability at js-yaml #183

Closed
em0ney opened this issue Apr 16, 2019 · 5 comments
Closed

Code injection security vulnerability at js-yaml #183

em0ney opened this issue Apr 16, 2019 · 5 comments

Comments

@em0ney
Copy link

em0ney commented Apr 16, 2019

Hi there,

Thanks for making cosmiconfig!

You have a dependency on js-yaml@3.13.0. Please see this report of a High severity vulnerability in this module.

https://www.npmjs.com/advisories/813

@em0ney em0ney changed the title Code injuection security vulnerability at js-yaml Code injection security vulnerability at js-yaml Apr 16, 2019
@olsonpm
Copy link
Contributor

olsonpm commented Apr 16, 2019

thanks for the report. I'll have time to take a look this weekend.

It's the second security issue we've had with that library within a short time frame though, I'll probably check out alternative libraries while I'm at it.

@em0ney
Copy link
Author

em0ney commented Apr 16, 2019

I might open up a PR for this one if there is a chance you can get it merged this week

@olsonpm
Copy link
Contributor

olsonpm commented Apr 16, 2019

if your issue is time sensitive then you should fork the repo and depend on that in the meantime. I assume this issue will be addressed within the next couple weeks, but we can't promise anything.

@sudo-suhas
Copy link
Contributor

Removing the lock file and a clean install would pull in the latest version of js-yaml. So for anybody who wishes to fix this issue immediately, there is no need to wait for a fix from this library(we'll do it though).

@olsonpm
Copy link
Contributor

olsonpm commented Apr 20, 2019

Alright, here's what I found.

js-yaml has three advisories raised on npm

Two of them are irrelevant to cosmiconfig because the risk was
associated with load. Cosmiconfig uses safeLoad

However because of the existence of load vs safeLoad, and js-yaml providing the
functionality to parse js functions and regexes in load, I think it's safe to assume more
advisories will be raised which are irrelevant to Cosmiconfig.

I found two other yaml parsing libraries that seem reliable but neither support node v4

yaml requires v6+ (seems more actively maintained than yaml-js)
yaml-js requires v8+

Both libraries

  • have no security advisories raised
  • don't parse js functions or regexes and thus don't have a load vs safeLoad concept

Unfortunately I think we're stuck bumping js-yaml versions every time an advisory is raised for the
duration of cosmiconfig v5 which will always support node v4. We should probably switch to
yaml when we bump node versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants