search

webpack / loader-utils

utils for webpack loaders
MIT License
766 stars 185 forks source link

Security Vulnerabilities issue #216

Closed kundarsowjanya closed 1 year ago

kundarsowjanya commented 2 years ago

Hi, I'm not using loader-utils directly in my package.json, but may be it has transitive dependency, and currently I'm using react-script v5, but now I'm getting Security Vulnerabilities issue as

A Regular expression denial of service (ReDoS) flaw was found in Function interpolateName in interpolateName.js in webpack loader-utils 2.0.0 via the resourcePath variable in interpolateName.js

and also I ran npm ls loader-utils, so the result is:

-- react-scripts@5.0.1 +-- @pmmmwh/react-refresh-webpack-plugin@0.5.8 |-- loader-utils@2.0.2 +-- @svgr/webpack@5.5.0 | -- loader-utils@2.0.2 deduped +-- babel-loader@8.2.5 |-- loader-utils@2.0.2 deduped +-- file-loader@6.2.0 | -- loader-utils@2.0.2 deduped +-- react-dev-utils@12.0.1 |-- loader-utils@3.2.0 -- resolve-url-loader@4.0.0 +-- adjust-sourcemap-loader@4.0.0 |-- loader-utils@2.0.2 deduped `-- loader-utils@2.0.2 deduped

could you please give me the suggestion how to fix this.

Thanks Sowjanya

alexander-akait commented 2 years ago

loader-utils 2.0.0 is deprecated, please ask developers to update to v3 (even more, you don't need in most of cases loader-utils 2.0.0, because most of feature were moved to webpack itself)

Donhv commented 2 years ago

loader-utils 2.0.0 is deprecated, please ask developers to update to v3 (even more, you don't need in most of cases loader-utils 2.0.0, because most of feature were moved to webpack itself)

i see webpack have no plan to update loader-utils version. they still use v2. i tried to force use v3 by resolutions but got error loaderUtils.getOptions

kundarsowjanya commented 2 years ago

Hi, even after using v3 still getting same error

A Regular expression denial of service (ReDoS) flaw was found in Function interpolateName in interpolateName.js in webpack loader-utils 3.0.0 via the resourcePath variable in interpolateName.js

jenilG commented 2 years ago

We are also facing the same issue with 3.0.2

alexander-akait commented 2 years ago

I can't reproduce it using v3:

akait@akait-notebook:~/IdeaProjects/css-minimizer-webpack-plugin$ npm i loader-utils

added 1 package, and audited 1102 packages in 2s

156 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
akait@akait-notebook:~/projects/project$ npm audit
found 0 vulnerabilities
akait@akait-notebook:~/projects/project$
alexander-akait commented 2 years ago

And also I think there is a problem with a security tool, becaue I can't reproduce ReDos using https://github.com/webpack/loader-utils/blob/master/lib/interpolateName.js#L51

alexander-akait commented 2 years ago

So if somebody have an example of this exploit - PR welcome or write me on sheo13666q @ gmail.com, I am glad to fix it, but can't find a way how it is possible to use reproduce (that is why I think there is a problem with security tools)

mdlkumaran commented 2 years ago

Facing same issue

npm audit (Use node --trace-warnings ... to show where the warning was created)

npm audit report

loader-utils ≤ 3.2.0 Severity: high A Regular expression denial of service (ReDoS) flaw was found in Function interpolateName in interpolateName.js in webpack loader-utils 2.0.0 via the resourcePath variable in interpolateName.js. - https://github.com/webpack/loader-utils/blob/d9f4e23cf411d8556f8bac2d3bf05a6e0103b568/lib/interpolateName.js#L38,https://github.com/webpack/loader-utils/blob/d9f4e23cf411d8556f8bac2d3bf05a6e0103b568/lib/interpolateName.js#L83,https://github.com/webpack/loader-utils/issues/211 fix available via npm audit fix --force Will install @angular-devkit/build-angular@13.0.4, which is a breaking change node_modules/loader-utils @angular-devkit/build-angular >=13.1.0-next.0 Depends on vulnerable versions of loader-utils node_modules/@angular-devkit/build-angular

2 high severity vulnerabilities

To address all issues (including breaking changes), run: npm audit fix --force

alexander-akait commented 2 years ago

Please avoid duplicate, because it doesn't help, thank you

v-pasha2 commented 2 years ago

is there any solution for this i am seeing this for version 3.2.0 as well ?

angularDevEng commented 2 years ago

any news?

alexander-akait commented 2 years ago

Please read my comments above, looks like it is false positive in security system, please report them

JSMike commented 2 years ago

Disregard, comment meant for #212

alexander-akait commented 2 years ago

@JSMike Thank for CVE-2022-37601, I will fix it, but the original issue about the security problem in interpolateName.js, and file and line is wrong

JSMike commented 2 years ago

@alexander-akait ah ok, sorry, I just assumed it was the same issue, I'll move my comments over to #212

pratheeshp007 commented 2 years ago

@alexander-akait Is ReDoS possible on this line https://github.com/webpack/loader-utils/blob/master/lib/interpolateName.js#L93

What if the line number was wrong in the report ?

alexander-akait commented 2 years ago

@pratheeshp007 Can you provide example of usage (exploit)? Because it is custom regexp and any utils which support RegExp as options can be affected this, I don't think it can be marked as exploit

LucasLopesr commented 2 years ago

is it a false positive?

alexander-akait commented 2 years ago

Sounds like yes, I tried to connect with administrators of security database, but no luck - no example of usage, no PoC and etc, the only found security problem was fixed in v2 here https://github.com/webpack/loader-utils/releases/tag/v2.0.3 (look at commits and you find where it was)

jeran-urban commented 2 years ago

Hello, left comments on other Issues, this Issue Looks Open, so here is the information for fixing both of the vulnerabilities shown in the OSS index:

The Vulnerabiliies:

The Problem

The Fix

ex of fix:

I believe this would mitigate the findings

jeran-urban commented 2 years ago

Please let me know if I can be of help here, my own app would benefit from the updates as well for security issues that I am currently handling

alexander-akait commented 2 years ago

@jeran-urban Feel free to send a PR with fixes

jmprentice commented 2 years ago

Have you evaluated this line as the possible culprit? https://github.com/webpack/loader-utils/blob/52cd134fc94721f62c9c882651aad24f33e6d6a8/lib/interpolateName.js#L79

I ran the Regex on that line through an online ReDoS checker and it came out vulnerable.

I believe this can be fixed by changing the Regex to /\[(?:([^\[:\]]+):)?(?:hash|contenthash)(?::([a-z]+\d*))?(?::(\d+))?\]/gi

The interpolateName.js tests pass with this change, but I don't know enough about what this section of your code is doing to be sure it wouldn't create any problems. If this seems like it will fix the ReDoS issue without breaking anything I can submit pull requests for the three versions. Our projects need versions 2.x and 1.x (loader-utils is a transitive dependency in multiple packages).

alexander-akait commented 2 years ago

@jmprentice Thank you, feel free to send a PR

alexander-akait commented 2 years ago

But not sure about https://github.com/advisories/GHSA-hhq3-ff78-jv3g, I really don't see any problems there

alexander-akait commented 2 years ago

Even more https://github.com/advisories/GHSA-hhq3-ff78-jv3g is refering on https://github.com/webpack/loader-utils/issues/211, but https://github.com/webpack/loader-utils/issues/211 is about another problem and it was fixed https://github.com/webpack/loader-utils/pull/217 and backported

jmprentice commented 2 years ago

My contributions are being rejected by EasyCLA, but you can see the change in the PR and should feel free to use it if it works.

alexander-akait commented 2 years ago

WIP, thank you anyway

alexander-akait commented 2 years ago

Fixed and released https://github.com/webpack/loader-utils/releases

abhisheknigam87 commented 2 years ago

I can still find this warning with v3.2.1: [CVE-2022-37603] A Regular expression denial of service (ReDoS) flaw was found in Function interpolateName in interpolateName.js in webpack loader-utils 2.0.0 via the url variable in interpolateName.js.

alexander-akait commented 2 years ago

@abhisheknigam87 I think it is mistabe, becaise we fixed it :confused:

jeran-urban commented 2 years ago

Sorry for not getting back sooner and thank you for getting to this so quickly. So, if I understand this correctly, here is some information:

There is still an issue for https://ossindex.sonatype.org/vulnerability/CVE-2022-37599?component-type=npm&component-name=loader-utils that will need to be addressed though.

Hopefully this helps and I will look at getting a PR for CVE-2022-37599 asap, but may be awhile until I have the free time needed.

jeran-urban commented 2 years ago

so I believe changing line 83

from this: directory = resourcePath.replace(/\\/g, '/').replace(/\.\.(\/)?/g, '_$1');

to this: directory = resourcePath.replaceAll(/\\/g, '/').replaceAll(/\.\.(\/)?/g, '_$1');

should fix the issue: Ex of difference between replace and replace all and how the call to the Regex constructor behind the scenes on replace can be dangerous: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll#description

I am not positive that this will resolve the issue, but it should allow for a safer open ended regex run against an unknown string with the built in processing of replaceAll at least. If not, I would look at adding a timer wrapper around the evaluation to fail if it takes longer than x seconds as a catch all.

PR https://github.com/webpack/loader-utils/pull/227/ submitted.

jeran-urban commented 2 years ago

to update the thread, on PR #227, the PR has been closed given the following information:

from looking at the various tickets, issues, and vulnerabilities, there are 3 vulnerabilities that have been discussed:

lastly

I did not see an issue with the current code either as there are no wildcard characters that are exploitable and this regex itself is simple and has no known redos issues currently either (in reference to line 83 as defined in the finding). But given that as of the day I posted this PR, OSSIndex still showed this as a valid finding, as was Snyk, NVD, etc. and as of yesterday at 11:22am cst, multiple security scans were still showing this as a finding as well.

Given that, there is a difference behind the scenes for replace vs. replace all, that I was hoping this would address, and would address the remaining open finding from the various security agencies that were still reporting this as an active issue.

But as of now, the scans we are running show this package as compliant and the finding appears to be considered addressed by OSS (not the other agencies yet). I already do have tickets open to Snyk, NVD and Mitre for the CVE to update these to show the issues as fixed, and have added this CVE to those tickets, not just the CVE-2022-37603 issue.

Given all of that, as of now, this PR does not seem to be needed and there does not appear to be an issue with the code, as the parts replaceAll targets as issues for replace are not being used here.

It may take time for the various security tools to update and not show these findings as valid, (and correct me if I am wrong @alexander-akait ), but as long as you are on the latest versions(1.4.2, 2.0.4, 3.2.1), you should be able to site this issue as verification that these are false positives. Thank you loader-utils team for addressing this so quickly! my team appreciates it a lot!

jeran-urban commented 2 years ago

updates with fixes posted to github advisory via PRs listed above, once accepted, those should show the recommended resolution on npm audit commands from now on

jeran-urban commented 2 years ago

Snyk has updated their documentation for affected versions, closed the tickets, and is now reporting no vulnerabilities for versions 1.4.2, 2.0.4, and 3.2.1!

sjorsverhoef commented 2 years ago

Nice work!

LucasLopesr commented 2 years ago

nice work guys!

jeran-urban commented 2 years ago

the github advisory entries have been updated and now npm audit will show the relevant errors, and there is an auto fix available that npm audit fix will resolve

jeran-urban commented 2 years ago

NIST's NVD is showing as fixed for https://nvd.nist.gov/vuln/detail/CVE-2022-37601 and https://nvd.nist.gov/vuln/detail/CVE-2022-37603, ~still working on https://nvd.nist.gov/vuln/detail/CVE-2022-37599~.

jeran-urban commented 2 years ago

https://nvd.nist.gov/vuln/detail/CVE-2022-37599 has now been updated as well.

With these updates, Sonatype's OSS INDEX, NIST's NVD, NPM Github Advisory, and Snyk's Reports have all been updated and all known vulnerabilities have been handled and backported for all major versions. I think this issue should be good to consider closed. I still have pending CVEs to be updated, but those normally take awhile.

For anyone tracking this issue that still is showing vulnerabilities in their scans, please give it another 24 hours for the agencies to push to feeds, and other agencies to update from those feeds to rerun your scans, after that, you should no longer see these issues as long as you are on the correct versions.

Thank you again to the loader-utils team and please let me know if I can be of any help in future.

jeran-urban commented 2 years ago

as an aside, @alexander-akait, I left a comment regarding the regex in https://github.com/webpack/loader-utils/pull/227, may be able to simplify or refactor the regex to target differently for the replace functionality