karma-runner / karma-runner/karma

Replace ua-parser-js or pin current version

Open
#3,715 5 comments 7 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
12k
Forks
1.7k
PR merge metrics
No merged PRs in 30d

Description

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

1. https://github.com/karma-runner/karma/issues/3562 (regex catastrophic backtracking)
2. https://github.com/karma-runner/karma/issues/3583 (regex catastrophic backtracking)
3. https://github.com/karma-runner/karma/issues/3680 (regex catastrophic backtracking)
4. https://github.com/karma-runner/karma/issues/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): https://github.com/karma-runner/karma/blob/94cf15e8fa4420c8716998873b77f0c4f59b9e94/lib/helper.js#L9

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?

Contributor guide

Open the contributing guide

Research direction

Start at the ua-parser-js use in lib/helper.js and review the linked security reports. The issue presents several replacement and configuration options, so first confirm which direction maintainers want. Done means the chosen dependency or browser-name handling is changed and logging behavior is verified with the project's checks.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
security, testing-qa
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.