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

Replace ua-parser-js or pin current version #3715

Open
davidje13 opened this issue Oct 26, 2021 · 5 comments
Open

Replace ua-parser-js or pin current version #3715

davidje13 opened this issue Oct 26, 2021 · 5 comments

Comments

@davidje13
Copy link

davidje13 commented Oct 26, 2021

It seems ua-parser-js has been the source of several security issues:

  1. Potentially vulnerable dependency ua-parser-js 0.7.21 #3562 (regex catastrophic backtracking)
  2. Security issue: Please update ua-parser-js to 0.7.23 to fix CVE-2020-7793 #3583 (regex catastrophic backtracking)
  3. Security issue with ua-parser-js dependency #3680 (regex catastrophic backtracking)
  4. ua-parser-js hijacked with a miner #3713 (malware compromise)

The catastrophic backtracking issues of course aren't much of a problem for Karma, but the recent malware compromise certainly is! Karma represents a large chunk of the library's usage (according to NPM, Karma represents ~25% of ua-parser-js's downloads), and since a version range import is being used, it means that Karma may have unwittingly been a big contributor to spreading the latest malware.

Yet the library is only used in a single location for a very minor purpose (constructing a "friendly" name for browsers to show in the logs; as far as I can tell, this is an undocumented internal function with no particular promises about its API):

exports.browserFullNameToShort = (fullName) => {

Maybe it's time to swap that function out for a home-grown simplified version? I think there are a few options:

  • handle just the UAs for browsers with supported launchers, and the rest can use the fallback full version string
  • don't try to create friendly names for any browsers; just use the full version string for everything
  • continue using ua-parser-js, but pin a specific version (0.7.30) instead of a range (^0.7.30) to at least avoid any future compromises being auto-fetched (and perhaps worth doing a quick review of the current code for other issues, if going with this option)
  • any of the above, plus make the function configurable (so that users can provide their own full-user-agent -> nice-name function, which could of course use any version of ua-parser-js if the user wants it). If doing this, my preferred choice would be to have the default just pass through the raw UA, removing the need for the dependency and keeping the code simple.

Do the maintainers have any preferences here?

@jginsburgn
Copy link
Member

Thanks for bringing this up. I think a home-grown version is the way to go. Can you send a PR?

@davidje13
Copy link
Author

sure; will take a look at it this weekend

@jginsburgn
Copy link
Member

Sounds great!

@davidje13
Copy link
Author

Created a PR

@LucasLopesr
Copy link

any news?

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

3 participants