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

Dependency on specific version of ua-parser-js prevents vulnerabilities in downstream projects from being fixed #2009

Closed
2 of 10 tasks
mattwelke opened this issue Jan 29, 2023 · 7 comments

Comments

@mattwelke
Copy link

mattwelke commented Jan 29, 2023

Issue details

I was addressing a Dependabot security vulnerability but it said that my project could not update to a fixed version of the dependency in question, ua-parser-js, because browser-sync requires a specific version of it:

updater | INFO <job_587648674> Requirements update strategy bump_versions
updater | INFO <job_587648674> The latest possible version that can be installed is 1.0.2 because of the following conflicting dependencies:
updater | <job_587648674> 
updater | <job_587648674>   browser-sync@2.27.11 requires ua-parser-js@1.0.2
updater | <job_587648674>   No patched version available for ua-parser-js
updater | INFO <job_587648674> The earliest fixed version is 1.0.33.
updater | INFO <job_587648674> Finished job processing

Steps to reproduce/test case

  1. Create NPM project that uses browser-sync

  2. Add dependency on browser-sync@2.27.11 to project. Observe warning message: 2 high severity vulnerabilities

  3. Run npm audit fix. Observe error:

    ua-parser-js  0.8.1 - 1.0.32
    Severity: high
    ReDoS Vulnerability in ua-parser-js version  - https://github.com/advisories/GHSA-fhg7-m89q-25r3
    fix available via `npm audit fix --force`
    Will install browser-sync@2.27.5, which is a breaking change
    node_modules/ua-parser-js
      browser-sync  >=2.27.6
      Depends on vulnerable versions of ua-parser-js
      node_modules/browser-sync
    
    2 high severity vulnerabilities
    

Please specify which version of Browsersync, node and npm you're running

  • Browsersync [2.27.11]
  • Node [v18.12.1]
  • Npm [9.1.3]

Affected platforms

  • linux
  • windows
  • OS X
  • freebsd
  • solaris
  • other (please specify which)

Browsersync use-case

  • API
  • Gulp
  • Grunt
  • CLI

If CLI, please paste the entire command below

n/a

CLI is the way it's used in my project but this issue comes up before it is used as a CLI.

@tkoop
Copy link

tkoop commented Feb 13, 2023

Please fix this. My npm audit is complaining about high severity vulnerabilities.

@ojvribeiro
Copy link

This can be closed since it was fixed by #2007.

@shakyShane
Copy link
Contributor

Fixed in 2.27.12 - thanks for your patience :)

@mattwelke
Copy link
Author

The change made in #2007 doesn't address this problem because it just updates the dependency to a new specific version. Using a version range would allow consumers of this library to determine the exact version used, which would allow them to address security vulnerabilities in their own projects without waiting for projects in the middle of the dependency chain, like this one, to be updated.

@shakyShane shakyShane reopened this Feb 24, 2023
@shakyShane
Copy link
Contributor

@mattwelke thanks! I was too eager in closing this - I saw 'ua-parser-js' title and looked no further, my apologies 🙏🏻

@mattwelke
Copy link
Author

mattwelke commented Feb 24, 2023

No problem. I think this is a controversial opinion I have here too. It's definitely something the maintainers should all think about and agree on. I see the resolution to this issue being either to close it with a new PR that switches to depending on a range of versions (usually patch), or to close it as a "won't fix" because the maintainers choose to continue pinning to an exact dependency. There are pros and cons to both approaches and both are valid.

Either way, I'm glad people can address this immediate security vulnerability. Thanks!

shakyShane pushed a commit that referenced this issue Feb 27, 2023

Verified

This commit was signed with the committer’s verified signature.
nlf nlf
@shakyShane
Copy link
Contributor

browser-sync@2.28.1 just uses ^, let's see how it goes :)

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

4 participants