Vulnerability in url-regex indirect dependency

[silverwind]

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ url-regex                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ fomantic-ui                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ fomantic-ui > gulp-concat-css > rework-import > url-regex    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1550                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

All these dependencies look pretty unmaintained to me so I think the best course of action would be to look for alternatives to gulp-concat-css.

[PMudra]

Some additions:

• gulp-concat-css: 2 years ago
• rework-import: 5 years ago
• url-regex: a year ago

Upstream issue: kevva/url-regex#70

This issue is fixed in my maintained and modern version of this package at https://github.com/niftylettuce/url-regex-safe. You should be able to switch from url-regex to url-regex-safe now.

[ceisele-r]

Well this would mean that gulp-concat-css would have to depend on a forked rework-import package that depends on https://github.com/niftylettuce/url-regex-safe. Since they all seem to be unmaintained, none of them will probably switch to any other dependency meaning as @silverwind supposed the way to go is probably replacing gulp-concat-css.

[gilquintana]

One question: do you have an estimated date for the attencion of this issue?

[lubber-de]

One question: do you have an estimated date for the attencion of this issue?

Whenever somebody volunteers to maintain new packages or rewrite the dependency code to be directly included into the core

[phiberber]

One question: do you have an estimated date for the attention of this issue?

Whenever somebody volunteers to maintain new packages or rewrite the dependency code to be directly included in the core

There's a branch that can be used that uses the URL regex safe dependency. I can create a PR using it if you want me to.

https://github.com/Cj-bc/gulp-concat-css/tree/use_url-regex-safe_rework-import

[lubber-de]

There's a branch that can be used that uses the URL regex safe dependency. I can create a PR using it if you want me to.

https://github.com/Cj-bc/gulp-concat-css/tree/use_url-regex-safe_rework-import

Thanks, you are welcome to provide a proper PR.

[phiberber]

After looking on it for a while, url-regex-safe doesn't seem to be a good option as it uses node-re2 (A node-gyp wrapper for Google re2). Would be nice if someone could find an alternative for url-regex. Also, gulp-concat-css is only used when packing the CSS, I'm not that good at hacking but I don't think that it would be possible to exploit that vulnerability. Then there should be no need to worry about it.

[silverwind]

I too think a C++ dependency like node-re2 is a no-go because it limits portability and is very prone to break in future Node.js versions. I'm sure there must be better alternatives.

[onelifenyc]

I just wanted to bring up the fact that this issue is also found in the following deps:

+

I didn't find any issue reported about it - I am sorry for tagging it along here. Let me know if I should open a new issue.

https://npmjs.com/advisories/1631
https://npmjs.com/advisories/1677

[MeFisto94]

I am a nodeJS beginner, so bear with me if my proposal/thinking is wrong.

1. Besides fixing the issues themselves, since the issues (at least the gulp related ones) never make it into the dist-files, it is some kinda false-positive for downstream projects.
2. It's furthermore less risky, because developers typically control the input to gulp in some form anyway.
3. Couldn't all gulp dependencies be moved into the devDependencies field anyway, because there isn't any part of fomantic-ui that makes it into the runtime? (besides the distribution, ofc). I tried that locally, and it gets rid of the issues within npm audit, I think (only hiding them, ofc).
4. Extending by that, shouldn't end-users also add fomantic-ui as dev dependency, given that they only need it to generate the dist?

On topic of gulp-dedupe, the last version has been pushed 7 years ago and it's 50 SLOC, I wonder why no-one has just forked it and accepted the dependabot PR bumping diff 3 major versions. Shouldn't be too much of a maintenance burden, but at the same time maybe there are maintained deduping alternatives.

[lubber-de]

gulp5 was upgraded by #3032
all other plugings have been upgraded by #3047

The remaining/new 2 moderate warnings will be fixed in FUI 2.10.0 when node 12 is dropped