High node-forge security vulnerability

[paulbrimicombe]

🐛 bug report

• node-forge < 0.10.0 has a high severity security vulnerability

🎛 Configuration (.babelrc, package.json, cli command)

package.json:

{
  "dependencies": {
     "parcel": "^1.12.4"
  }
}

🤔 Expected Behavior

No audit failures

😯 Current Behavior

Running npm install and then npm audit gives the following output:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Prototype Pollution in node-forge                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ node-forge                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >= 0.10.0                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ parcel                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ parcel > node-forge                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1561                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 1 high severity vulnerability in 746 scanned packages
  1 vulnerability requires manual review. See the full report for details.

💁 Possible Solution

Upgrade to node-forge >= 0.10.0

🌍 Your Environment

Software	Version(s)
Parcel	1.12.4
Node	12.16.1
npm/Yarn	npm 6.14..8
Operating System	MacOS

[DeMoorJasper]

This has already been reported, and fixed in Parcel 2.

This is also not a high vulnerability, npm just flagged this incorrectly... as usual

[paulbrimicombe]

Thanks for the quick response @DeMoorJasper

This has already been reported, and fixed in Parcel 2.

AFAIK parcel 2 is currently not currently stable — npm lists it as being in the first beta and so this "fix" is not currently released.

Do you have a deprecation plan for Parcel 1? You will have to support both versions for a period of time while users migrate. It is good practice to apply security fixes / patches for a grace period while people migrate their code.

This is also not a high vulnerability, npm just flagged this incorrectly... as usual

Hmm. Synk has it listed as high as well — https://snyk.io/vuln/SNYK-JS-NODEFORGE-598677 . What is your source for saying it has been "flagged incorrectly"?

Even if the vulnerability is not high it's good practice to avoid all security vulnerabilities if possible.

I've upgraded node-forge and run all of the parcel tests and everything seems fine. If I raise a PR has is there any chance it will be merged?

[DeMoorJasper]

@paulbrimicombe my source for it not being as severe as flagged is the changelog of node-forge, it was an unused legacy util... we definitely don't use it in Parcel. Hopefully npm will start doing proper code analysis at some point (now that they've been acquired by GitHub) to stop wasting time with these false positive warnings.

The chance of something getting merged into Parcel 1 is small, @devongovett is the only person who can push to npm so he's the only one who can fix this. I've brought this up internally but haven't received any response to that so seems unlikely.

The CI for Parcel 1 is very broken, so a PR wouldn't really help a lot unfortunately. We kinda messed up when trying to merge Parcel 1 and Parcel 2 into one branch.

[paulbrimicombe]

The chance of something getting merged into Parcel 1 is small, @devongovett is the only person who can push to npm so he's the only one who can fix this. I've brought this up internally but haven't received any response to that so seems unlikely.
The CI for Parcel 1 is very broken, so a PR wouldn't really help a lot unfortunately. We kinda messed up when trying to merge Parcel 1 and Parcel 2 into one branch.

We have other issues with Parcel 1 at the moment that are causing us problems (e.g. #2921 which has other people asking for the fix in 1.x). Is there really no way to get this fixed? Can I help with fixing the 1.x build pipeline?

[torotil]

The CI for Parcel 1 is very broken, so a PR wouldn't really help a lot unfortunately. We kinda messed up when trying to merge Parcel 1 and Parcel 2 into one branch.

Is it correct that this means Parcel 1 is no longer supported?

[mmikhalko]

Please provide the fix for v1

[sombreroEnPuntas]

This is a high severity vulnerability. Projects should ALWAYS aim to reduce attack surface. A little hole here, an other one there... and one ends up trying to navigate the seas of the internetz on a colander.

[johannchopin]

Why is this issue closed? Maybe it is not so dangerous at all but it definitely feels better to not see this kind of alert message on your GitHub project. So is there some works on it? Could we help in any way?

[mmikhalko]

Please pay attention at the comments above, community wants a fix of this problem!

[torotil]

I don’t think putting pressure on the maintainers to make them maintain Parcel v1 will work. It seems their ressources are bound with v2 so it would need someone else to step up.

@paulbrimicombe could you give us pointers at what would help you to get out a patch release for v1? Is there some way to donate to the project? Would creating a PR that fixes the CI build help?

[paulbrimicombe]

I don’t think putting pressure on the maintainers to make them maintain Parcel v1 will work. It seems their ressources are bound with v2 so it would need someone else to step up.

I don't think it's unreasonable to expect maintainers to apply security fixes to the only stable version of their library (see https://www.npmjs.com/package/parcel for a list of the current versions – v2 is still in beta).

@paulbrimicombe could you give us pointers at what would help you to get out a patch release for v1? Is there some way to donate to the project? Would creating a PR that fixes the CI build help?

Unfortunately I'm not actually a maintainer / contributor to this project so I have no idea what the actual problem is. I have offered to help fix the delivery pipeline issues but I had no response. @DeMoorJasper do you have any suggestions?

[torotil]

I don't think it's unreasonable to expect …

On the contrary: It’s an open source project so the only thing you can expect is that the code is public, which it is. You can wish for and suggest more though and these wishes get fulfilled so regularly that it has become an expectation for you.

I have seen many maintainers becoming too frustrated and quit because of such issues in the past years. I’d like to avoid that.

[paulbrimicombe]

On the contrary: It’s an open source project so the only thing you can expect is that the code is public, which it is. You can wish for and suggest more though and these wishes get fulfilled so regularly that it has become an expectation for you.

I have seen many maintainers becoming too frustrated and quit because of such issues in the past years. I’d like to avoid that.

This would be true if the project was in a different state. It is completely understandable that projects that are reaching end-of-life have issues similar to this. That is not the case for parcel – the team is actively still working on the project and would like more users to use the library. This is not going to happen if users do not have confidence that the library has long-term support for stable versions.

Multiple people have offered their time to fix this issue – I raised a PR to fix it directly. I (and other people) then offered to help fix whatever problems there are in the background that mean that parcel v1 can no longer be built. This has been met with complete silence from the project maintainers.

I would have a hard time encouraging anyone to use parcel for anything other than hobby use given the state of this issue.

[DeMoorJasper]

@paulbrimicombe Parcel 2 is pretty much as stable as Parcel 1 at this point we just want it to be very stable before a rc or stable version, we also have some api changes lined up so we can't release a stable before those are in. It's already used in production at some large corporations...

So unless you're writing custom plugins or using the js api parcel 2 should work just fine.

The PRs I have seen for updating this dependency always had failing tests or were based on the wrong branch.

[paulbrimicombe]

The PRs I have seen for updating this dependency always had failing tests or were based on the wrong branch.

Thanks for the reply @DeMoorJasper – If I raise a PR is there any chance that it will actually get merged? I'm very happy to do the work if that's the case. I'm guessing that master is the v1 branch – please correct me if I'm wrong.

You say that existing PRs have breaking tests – the current HEAD of the master branch has failing tests. Would we be expected to fix whatever those broken tests are before we can get any other changes merged?

https://github.com/parcel-bundler/parcel/runs/282640521

P.S. Some thoughts on parcel 2:

• This issue was raised in mid-October at which point parcel 2 was in alpha (as I recall).
• If there are breaking API changes on the way then I'd be reluctant to pick up the work to migrate to parcel 2 not knowing how much more migration work will be required.
• Please bear in mind that people can't just suddenly drop everything and migrate when there are issues with v1 – they will have other priorities.

[DeMoorJasper]

@paulbrimicombe the linked run looks good enough, not everything is green but most is. Tests were always flaky on Parcel 1 due to filesystems being terrible to test on, especially windows.

[devongovett]

You do realize that this isn't an actual issue right? You have to use nodeforge.util.setPath in order to trigger the vulnerability, which parcel does not. These npm warnings just create work for maintainers in most cases due to people complaining in issues like this one.

If you really cannot ignore the warning, then there are ways around it. You can use yarn resolutions or npm-force-resolutions to bump the version of nodeforge yourself for example.

Parcel 1 is in maintenance mode at this point and has been for some time. There has not been a release since October 2019, and we do not plan any future releases. Parcel 2 is much more stable, and is actively developed. I would encourage you to migrate when you can. It should be very straightforward in most cases. And as Jasper mentioned, Adobe and Atlassian are already running Parcel 2 in production.

I realize this may be frustrating, but we have a small core team with limited time. Parcel 2 has already been in development for a couple years at this point, so all of our energy is focused on that. We can do small fixes to Parcel 1 if something is completely broken, but this issue feels like a relatively minor inconvenience with a workaround available so I'm not terribly inclined to spend time on it.