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

Create a c3.11.3 to resolve vulnerability in dependency ansi-html #3929

Closed
ghost opened this issue Oct 8, 2021 · 5 comments
Closed

Create a c3.11.3 to resolve vulnerability in dependency ansi-html #3929

ghost opened this issue Oct 8, 2021 · 5 comments

Comments

@ghost
Copy link

ghost commented Oct 8, 2021

Bug report

Users of Angular 11 and 12 are vulnerable to ReDoS because of the package (ansi-html) in 3.11.2. There is a fix which is being used in the later versions of webpack-dev-server which are not compatible for Angular 11/12. Instead of using ansi-html package, use ansi-html-community version 0.0.8.

Simple fix saving 1000s of customers from a potential attack.

[https://snyk.io/vuln/SNYK-JS-ANSIHTML-1296849]

Actual Behavior

vulnerable to ReDoS

Expected Behavior

no vulnerable packages

How Do We Reproduce?

{
"name": "webpack-dev-server",
"version": "3.11.2",
...
"dependencies": {
"ansi-html": "0.0.7",

Please paste the results of npx webpack-cli info here, and mention other relevant information

@alexander-akait
Copy link
Member

webpack-dev-server v3 is outdated and don't accept updates, there are other security problems, please ask angular team to update dev server, updating ansi-html is not solve security problems

@G-Rath
Copy link

G-Rath commented Oct 25, 2021

@alexander-akait I understand that position and generally agree that updating is the ideal way to go however @rails/webpacker is currently locked into using v3 of webpack-dev-server which currently is resolved in their unreleased v6 release; however there isn't a timeframe on when that'll actually get released and it's not something that I think is high priority in the rails team due to focuses being elsewhere (including v7 of Rails, which is meant to include a whole new decoupling of the js side of things as a move away from @rails/webpacker so we might even see the v6 release).

Additionally, the ansi-regex team have pushed back on backporting the fix to older versions. As I said, while I understand and generally agree with the position (and the whys behind it), it does leave us in the Rails community (and surroundings) in a position where we can't really do anything and that is very unlikely to resolve itself for at least a few years (especially when factoring in the time it will take to upgrade applications to new majors of things).

While I understand the actual vulnerability is not critical, it's still something that is visible to all forms of people (e.g pentesters, govt heads of security) that tend to require us to fill out long reports, give justifications, and explain why something is or isn't a vulnerability (and typically this has to be done per client, each with their own slightly different and unique set of requirements) - so overall it's a lot easier for me if I can get it patched most of the time.

I currently have two ways forward that I'd like to explore:

  • seeing if there is a way to allow @rails/webpacker v5 to support both v3 and v4 of webpack-dev-server, that I can get landed in a minor or patch release
  • seeing if I can get webpack-dev-server v3 to pull in a "secure" version of ansi-regex.

Both of these obviously require a level of interaction from their respective teams to review, merge, and release the PR which brings me to my question for you: if I was to try and come up with a patch for v3 of webpack-dev-server that would resolve this, keeping it as small as possible and without being a breaking change, would you be open to reviewing and releasing that?

I would prefer doing a patch for webpack-dev-server since this does affect other ecosystems like Laravel (via laravel-mix), but would like to know if it's something you're open to before I sink a lot of time into it.

@alexander-akait
Copy link
Member

alexander-akait commented Oct 26, 2021

@G-Rath got it, what I should review and merge?

@G-Rath
Copy link

G-Rath commented Oct 26, 2021

@alexander-akait I'll try and get PRs for you to review this week.

I expect there to be two (since there are two vulnerabilities):

  1. ansi-html - that package is actually unmaintained, but there's a published fork that patches the vulnerability so it should just be a matter of switching to using ansi-html-community
  2. ansi-regex will probably be a bit more work, since it's pulled in from strip-ansi and v3 of wds supports Node v6 which doesn't align with a patched version of strip-ansi or ansi-regex that still supports Node v6, so I'll probably have to inline those dependencies - luckily they're only a few lines of code each. (I may try appealing to the devs of ansi-regex again to see if we could get just a v4 release, which would mean I'd only need to inline strip-ansi, but I'm doubtful) actually this is being pulled in by yargs, so won't be doable without a lot of effort.

Thanks for being open to this ❤️

@G-Rath
Copy link

G-Rath commented Oct 26, 2021

@alexander-akait actually looks like there's already a PR open for replacing ansi-html: #3813

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

No branches or pull requests

2 participants