CVE With JSON5 v1.0.1

[ashleyfrieze]

Recommended upgrade to 1.0.2/2.2.2

https://github.com/advisories/GHSA-9c47-m6qq-7p4h , https://github.com/json5/json5/issues/199 , https://github.com/json5/json5/issues/295 , https://github.com/json5/json5/security/advisories/GHSA-9c47-m6qq-7p4h , https://nvd.nist.gov/vuln/detail/CVE-2022-46175
Evidence

This is a consequence of being on the v3.x of tsconfig-paths - the CVE is fixed in the later version of tsconfig-paths which uses [email protected]

[ljharb]

tsconfig-paths v3 depends on json5 v1, and v1.0.2 fixes the issue (which doesn't actually apply to eslint-plugin-import anyways, it's a false positive) so there's literally nothing we need to do here.

[ashleyfrieze]

@ljharb - it's easy to dismiss an issue for those reasons, but what you've done is damned us to transitive dependency scanning hell.

Were you to add a resolutions section in your package.json to force the transitive dependency to version 1.0.2, that would also help.

Similarly, if you were to remain up to date with the latest packages, this wouldn't happen.

Ignoring it on your end just means all your consumers with scanning software have to put up with the problem. You're expecting them to know that this is a false positive because the library's not really used.

[ljharb]

I really don't know what you mean. You have the freedom to trivially update your lockfile to the latest dependency - you can even npm install eslint-plugin-import or npm update and it will include it.

It is both unreasonable and unnecessary to expect maintainers to keep ranged dependencies up to date. You are responsible for keeping your entire dep graph up to date within the specified ranges, not random maintainers.

Separately, resolutions (which is yarn-only, but npm's overrides works the same) simply doesn't apply to consumers of your project, so no, adding it to this project wouldn't help anyone - YOU, however, can do this just fine in your app without bothering anyone else.

[ashleyfrieze]

So if I submit a PR with the 1.0.2 fix in, you'll reject it, and instead explain why I shouldn't need to have a clean dependency graph?

[ljharb]

tsconfig-paths v3 would need that fix, since we don't directly depend on json5.

You do need to have a clean dependency graph - it's just that the onus is on YOU to keep it so when updates are in-range, not on transitive maintainers.

[ashleyfrieze]

A few thoughts:

A lot of people use this library. That's a lot of people who may face the same issue I've faced, which is trying to track down the importance/otherwise of a transitive dependency and whether it's safe to ignore/override/resolutions my way out of it.

There are a lot of known issues in this package.json - it might be worth staying abreast of updates. If we all did that, there's a good chance that we don't have too much impact when real vulnerabilities, rather than false positives come along.

I made an experiment in PR #2771 to see if updating tsconfig-paths would help.

As far as I can tell, the unit tests pass... They may fail on the PR build.

[ljharb]

Unfortunately no, updating to v4 is a nonstarter; see #2447.

[ashleyfrieze]

Interestingly, you're on "tsconfig-paths": "^3.14.2" which does have [email protected] in it, which should suffice.

I appreciate the challenge of going for full backwards compatibility and how that stretches against dependency management. Thanks @ljharb for helping me get through the discussion to this final conclusion.