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

Security Vulnerabilities issue #216

Closed
kundarsowjanya opened this issue Oct 16, 2022 · 42 comments
Closed

Security Vulnerabilities issue #216

kundarsowjanya opened this issue Oct 16, 2022 · 42 comments

Comments

@kundarsowjanya
Copy link

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
Copy link
Member

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
Copy link

Donhv commented Oct 18, 2022

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
Copy link
Author

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
Copy link

jenilG commented Oct 18, 2022

We are also facing the same issue with 3.0.2

@alexander-akait
Copy link
Member

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
Copy link
Member

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
Copy link
Member

alexander-akait commented Oct 18, 2022

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
Copy link

mdlkumaran commented Oct 19, 2022

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
Copy link
Member

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

@v-pasha2
Copy link

v-pasha2 commented Oct 20, 2022

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

@angularDevEng
Copy link

any news?

@alexander-akait
Copy link
Member

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

@JSMike
Copy link

JSMike commented Oct 20, 2022

Disregard, comment meant for #212

@alexander-akait
Copy link
Member

alexander-akait commented Oct 20, 2022

@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
Copy link

JSMike commented Oct 20, 2022

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

@pratheeshp007
Copy link

@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
Copy link
Member

alexander-akait commented Nov 1, 2022

@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
Copy link

is it a false positive?

@alexander-akait
Copy link
Member

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
Copy link

jeran-urban commented Nov 3, 2022

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:

  • [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.

    • Description: 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.
  • [CVE-2022-37599] CWE-400: Uncontrolled Resource Consumption ('Resource Exhaustion')

    • Description: 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.

The Problem

  • The main issues around Regex DOS attacks is in badly formed queries that are not strict enough, and no exit condition involving a time limit for processing. If you ensure that the query is perfectly formed or at least add an exit condition based on a time limit (a few seconds) then this should resolve the issue. More information here: https://www.regular-expressions.info/redos.html#Handling%20Regexes%20Provided%20by%20The%20User. I believe this is still a legitimate vulnerability in v 3.x as well

The Fix

  • multiple options listed under the CWE listing (https://cwe.mitre.org/data/definitions/1333.html, under Potential Mitigations section)
  • update the regex to remove vulnerabilities
  • add a timeout wrapper to prevent a malicious regex string from causing failures, dos, or resource overloads

ex of fix:

I believe this would mitigate the findings

@jeran-urban
Copy link

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
Copy link
Member

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

@jmprentice
Copy link

Have you evaluated this line as the possible culprit?

/\[(?:([^:\]]+):)?(?:hash|contenthash)(?::([a-z]+\d*))?(?::(\d+))?\]/gi,

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
Copy link
Member

@jmprentice Thank you, feel free to send a PR

@alexander-akait
Copy link
Member

alexander-akait commented Nov 10, 2022

But not sure about GHSA-hhq3-ff78-jv3g, I really don't see any problems there

@alexander-akait
Copy link
Member

alexander-akait commented Nov 10, 2022

Even more GHSA-hhq3-ff78-jv3g is refering on #211, but #211 is about another problem and it was fixed #217 and backported

This was referenced Nov 10, 2022
@jmprentice
Copy link

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
Copy link
Member

WIP, thank you anyway

@alexander-akait
Copy link
Member

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

@abhisheknigam87
Copy link

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
Copy link
Member

@abhisheknigam87 I think it is mistabe, becaise we fixed it 😕

@jeran-urban
Copy link

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
Copy link

jeran-urban commented Nov 14, 2022

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 #227 submitted.

@jeran-urban
Copy link

jeran-urban commented Nov 15, 2022

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

  • [CVE-2022-37599] - a more recent high finding that has not yet been addressed in some systems (quoted as a problem against the resourcePath variable in this file) (no direct fix provided).

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
Copy link

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
Copy link

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
Copy link

Nice work!

@LucasLopesr
Copy link

nice work guys!

@jeran-urban
Copy link

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
Copy link

jeran-urban commented Nov 17, 2022

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
Copy link

jeran-urban commented Nov 17, 2022

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
Copy link

as an aside, @alexander-akait, I left a comment regarding the regex in #227, may be able to simplify or refactor the regex to target differently for the replace functionality

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