search

rollup / rollup-pluginutils

This package has moved and is now available at @rollup/pluginutils / https://github.com/rollup/plugins
45 stars 17 forks source link

Vulnerability coming from micromatch > braces #51

Closed XhmikosR closed 5 years ago

XhmikosR commented 5 years ago 
 Low             Regular Expression Denial of Service

 Package         braces

 Patched in      >=2.3.1

 Dependency of   rollup-plugin-babel [dev]

 Path            rollup-plugin-babel > rollup-pluginutils > micromatch >
                 braces

 More info       https://nodesecurity.io/advisories/786

...which is weird since I see you already target micromatch ^2.3.1. I've tried cleaning up local node_modules and package-lock.json and doing npm i, same thing.

Maybe the advisory has the wrong info?

XhmikosR commented 5 years ago

Ah, yeah, I need more coffee. 2.3.1 is the braces version not the micromatch version.

So, it seems this needs micromatch >= 3.1.6 which could break some things, although judging from their changelog it should work https://github.com/micromatch/micromatch/blob/master/CHANGELOG.md#300---2017-04-11

XhmikosR commented 5 years ago

The comment is there, don't be rude.

XhmikosR commented 5 years ago

You are still being rude.

lukastaegert commented 5 years ago

A RegExp DoS issue is breaking the Rollup eco system? In what universe would this be an issue for Rollup users? But I agree this needs to be fixed, Rollup's own builds are breaking as well.

Unfortunately there are good reasons we did no upgrade micromatch yet, in short #43. Will have another look if something can be done about the size issue, otherwise upgrade needs to be the way to go for now.

I just hope they finally get the 4.0 version going: micromatch/micromatch#137

XhmikosR commented 5 years ago

The problem is it breaks CI for many people. I've made a PR but since I can't test it thoroughly I'm gonna close it for now.

lukastaegert commented 5 years ago

I know, as it is breaking for us. I am on it, expect a resolution in the next hours.

lukastaegert commented 5 years ago

Should be resolved now but I was wrong about the size increase—the next version of Rollup WILL be 50% larger because micromatch@3 thinks it needs to have every stupid dependency out there. Seriously starting to look at minimatch again.

XhmikosR commented 5 years ago

Are you sure tslint needs to be in the dependencies ?

On Sat, Feb 16, 2019, 19:56 Lukas Taegert-Atkinson <notifications@github.com wrote:

Should be resolved now but I was wrong about the size increase—the next version of Rollup WILL be 50% larger because micromatch@3 thinks it needs to have every stupid dependency out there. Seriously starting to look at minimatch again.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rollup/rollup-pluginutils/issues/51#issuecomment-464367357, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVVtaHgzBF3kTcq_7F4r-UQwyUy6XVcks5vOEYxgaJpZM4a-3SP .

lukastaegert commented 5 years ago

Definitely not, fixed

phiphan148 commented 5 years ago

I got the same problem, how to fix it?