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

CVE-2020-28469 from parse-glob > glob-base > glob-parent #664

Closed
dalisoft opened this issue Jul 12, 2021 · 16 comments · Fixed by #927
Closed

CVE-2020-28469 from parse-glob > glob-base > glob-parent #664

dalisoft opened this issue Jul 12, 2021 · 16 comments · Fixed by #927
Assignees

Comments

@dalisoft
Copy link

Describe the bug
GHSA-ww39-953v-wcq6

To Reproduce
Steps to reproduce the behavior:

  1. Install htmlhint on your nodejs package
  2. Upload to GitHub (and enable security checks) or check via npm audit

Expected behavior
Secure code

Screenshots

Desktop (please complete the following information):

  • OS: Any
  • Browser: Any
  • Version: v0.15.1

Additional context

❯ yarn why glob-parent
yarn why v1.22.10
[1/4] 🤔  Why do we have the module "glob-parent"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "glob-parent@5.1.2"
info Has been hoisted to "glob-parent"
info Reasons this module exists
   - "workspace-aggregator-d96d4c71-52a9-4b9f-94d5-9e89242351ec" depends on it
   - Hoisted from "_project_#fast-glob#glob-parent"
   - Hoisted from "_project_#smartlint#eslint#glob-parent"
   - Hoisted from "_project_#smartlint#@stoplight#spectral#fast-glob#glob-parent"
+   - Hoisted from "_project_#lerna#@lerna#add#@lerna#command#@lerna#project#glob-parent"
info Disk size without dependencies: "76KB"
info Disk size with unique dependencies: "92KB"
info Disk size with transitive dependencies: "108KB"
info Number of shared dependencies: 2
=> Found "glob-base#glob-parent@2.0.0"
+ info This module exists because "_project_#smartlint#htmlhint#parse-glob#glob-base" depends on it.
info Disk size without dependencies: "28KB"
info Disk size with unique dependencies: "44KB"
info Disk size with transitive dependencies: "60KB"
info Number of shared dependencies: 2
✨  Done in 0.55s.
airlight on  master via ⬢ v16.4.2 
❯ 

I hope your'll fix this as soon as possible

@stale
Copy link

stale bot commented Sep 11, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the bot:stale Issue marked as stale because there was no activity label Sep 11, 2021
@aarongoldenthal
Copy link

After looking at the nature of this vulnerability, it's limited impact to htmlhint itself, but for those of us with compliance requirements to verify identified vulnerabilities are not an impact it is ongoing work, especially as it gets captured in more systems (NPM-1751, CWE-400), so it would be helpful if it could be removed.

It looks, though, like that means upgrading to something other than parse-glob, which is abandoned in place (no release in 6 years and no commits 2 years).

/app # npm audit

                       === npm audit security report ===


                                 Manual Review
             Some vulnerabilities require your attention to resolve

          Visit https://go.npm.me/audit-guide for additional guidance


  Moderate        Regular expression denial of service

  Package         glob-parent

  Patched in      >=5.1.2

  Dependency of   htmlhint [dev]

  Path            htmlhint > parse-glob > glob-base > glob-parent

  More info       https://npmjs.com/advisories/1751

found 1 moderate severity vulnerability in 717 scanned packages
  1 vulnerability requires manual review. See the full report for details.

@stale stale bot removed the bot:stale Issue marked as stale because there was no activity label Sep 11, 2021
@thedaviddias thedaviddias self-assigned this Sep 16, 2021
@Borosh
Copy link

Borosh commented Sep 23, 2021

This issue affects us too. I was trying to use the package in our project, but infosec won't let us, until you do something with that package. Do you know probably when is this going to happen?

@diegocr
Copy link

diegocr commented Nov 12, 2021

Any update on this please?

@josundt
Copy link
Contributor

josundt commented Nov 12, 2021

After updating all npm packages in our fairly large web application to latest versions, htmlhint is now the only package causing security audit warnings.

Any 3rd party library under healthy maintenance needs respond to newly discovered security vulnerabilities by releasing hotfixes within reasonable time. This security issue has existed for many months now, I sincerely hope a fix will be prioritized as soon as possible.

@nschonni nschonni changed the title Security issue CVE-2020-28469 from parse-glob > glob-base > glob-parent Nov 24, 2021
@nschonni nschonni pinned this issue Nov 24, 2021
@nschonni
Copy link
Contributor

It seems like the parse-glob package is no longer maintained, so the only way to resolve this is to replace the dependency.
Not sure if there is a new preferred library for that functionality

@diegocr
Copy link

diegocr commented Nov 24, 2021

fork -> rename -> fix -> release (i.e. parse-glob2) (?)

However, the author(s) of parse-glob seem the same than the micromatch library which looks like more actively maintained (last updated a month ago), dunno if that can be a proper replacement here, but given they're active around perhaps they will agree on transferring maintenance ownership for parse-glob.

/cc @jonschlinkert

@nschonni
Copy link
Contributor

@diegocr I don't think anyone here is offering to take over parse-glob or support a fork

@diegocr
Copy link

diegocr commented Nov 24, 2021

Hold my beer ;-)

Seriously, let's wait to hear from Jon and we will see the path forward then, if in the meantime you find a proper replacement and keen on just replacing parse-glob then that's up to you guys, the easier and swiftly this is handled the better.

@stuartbale
Copy link

The one area that uses parse-glob is here:

function getGlobInfo(target: string): {

It appears the purpose of this function is to extract out the 'base' and 'pattern' from a given file path.
I've tried to trace through how this is then used, but very quickly got lost in the internal workings of htmlhint.

I wonder if someone could confirm the usage of parse-glob, which may make it easier for the community to discover an appropriate up-to-date alternate that would provide equivalent functionality?

@aarongoldenthal
Copy link

The right answer may be to go the route that Tailwind CSS did and remove parse-glob since it's no longer supported, bring in is-glob and glob-parent directly since they are, add fill in any missing pieces.

@jdussouillez
Copy link

Any update on this ?

@joeyparrish
Copy link
Contributor

@josundt replaced the vulnerable module in #827, which has been stalled in review long enough to have merge conflicts. I rebased it and generated the JS binaries in #927.

If you need a fix now without waiting for HTMLHint to review and merge one of these two PRs, you may add my fork as a dependency to replace the latest HTMLHint release: github:joeyparrish/HTMLHint#1c3a7e8b.

@joeyparrish
Copy link
Contributor

Thanks, @josundt for the fix and @thedaviddias for reviewing and merging the PR!

@thedaviddias, please let us know when the v1.1.3+ is released with the fix, so we can all update our deps.

@stuartbale
Copy link

@joeyparrish - awesome work - I'm super pleased that this is finally resolved.

@thedaviddias
Copy link
Member

🎉 This issue has been resolved in version 1.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@coliff coliff unpinned this issue Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants