search

foundation / inky

Convert a simple HTML syntax into tables compatible with Foundation for Emails.
http://foundation.zurb.com/emails
673 stars 107 forks source link

Vulnerability in vinyl-fs used version #120

Closed vncdias closed 4 years ago

vncdias commented 4 years ago

Hello Inky maintaining team,

I started using Inky in a project and running npm audit I realised that there is a vulnerability in a dependency chain. Here is the audit result of project using inky@^1.3.7:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ braces                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=2.3.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ inky                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ inky > vinyl-fs > glob-stream > micromatch > braces          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/786                             │
└───────────────┴──────────────────────────────────────────────────────────────┘

I was looking for the best approach to fix that and seems that updating vinyl-fs to 3.0. will do the trick: every other 2. versions use a vulnerable version of braces at the end, and 3 doesn't. Since it's a major update I suppose the team need to check if there's some change to do to make it still working.

If possible, I can contribute with code too, since I'm interested on see this working in a project that have a vulnerabilities check at their pipeline.

jwmunn commented 4 years ago

I've run into this issue as well on my team since we have a dependency on Inky. I've opened a PR to propose a fix: PR https://github.com/foundation/inky/pull/121

joeworkman commented 4 years ago

This issue has been mentioned on Foundation Open Source Community. There might be relevant details there:

https://foundation.discourse.group/t/security-vulnerability-in-inky/2774/1

vncdias commented 4 years ago

Thanks for the PR @jwmunn Since I don't know the reason of using vinyl-fs and it's a major upgrade, I wasn't sure about the impacts of just bumping the version, but certainly that does the trick.

DanielRuf commented 4 years ago

We will upgrade the dependencies when we find time maintaining this project. Thanks for the heads up.