Update node-pre-gyp to fix prototype pollution attack #201
Conversation
|
Would appreciate this being merged in too! |
|
Thank you for the change @fenichelar! @es128 Please merge, our jenkins builds are crying 😢 . At least it's listed as a low severity though. |
|
Wanna bump it again to |
|
@wtgtybhertgeghgtwtg Done! |
|
Bump! Any status updates on this PR? 🙂 |
|
Friendly bump @es128 |
|
Sorry for the long delay. I'll work on updating node-pre-gyp now, but I won't be accepting this PR because of the baked-in version bump. The convention here is to do package version bumps as a separate commit by the maintainers, not in the PR. |
|
@es128 What will the version change be? I included it to emphasize that I think the version bump should be to 1.1.4 not 1.2.0 to ensure that this change is absorbed upstream as fast a possible. I understand that this release will also be dropping support for Node 0.12. |
|
Where do you expect the distinction between 1.1.4 and 1.2.0 to come in? Your dependencies are including fsevents via the I was planning to bump minor, not patch, just to create an easier delineation point of the breakage for 0.12, but I suppose that's still very arbitrary since we're breaking semver here anyway. |
|
The problem is that fsevents is used by so many packages. I don't directly use fsevents, but I'm sure some packages have included fsevents with the semver operator. Just like many people have included it as a required package on all OSs... Ideally, a security vulnerability should be fixed with a patch. But, yes, dropping support for Node 0.12 should be a major version change. |
This is not an actual security vulnerability for fsevents. The motivation is to stop the nagging of vulnerability scanning tools. |
|
@es128 Of course, but I think it should be treated the same as a security vulnerability in fsevents. Not everyone has enough time to dig into every vulnerability and truly understand how it affects them. So a known security vulnerability is a security problem, regardless of if it can actually be exploited. Just my 2 cents. So, what are your thoughts on the version? |
|
Chokidar is the primary consumer and I'm a maintainer of chokidar. I'm thinking major bump on fsevents, patch bump on chokidar. I'm not aware of significant usage that doesn't go through chokidar, so that should mitigate the problems of making the major bump at this layer. |
|
NPM shows 1128 public packages are dependent on fsevents (including chokidar). Not sure the levels of use of these packages though. There are a lot of React related packages. chokidar has 2823 dependents. Major here and patch in chokidar sounds reasonable. There are no great options. |
|
Any time frame when a new release will be added to npm? |
|
I understand that this is not a vulnerability for fsevents, but it would still be nice to see this released soon. |
|
Yeah, we have our own internal npm repository at work, and to onboard anything we have to run a package and its dependencies through vulnerability scanning, among other checks. Essentially this means that we cannot onboard any npm packages that depend on fsevents until this change is deployed. This is holding us up quite a lot - as we are currently stuck on outdated versions of Angular. If this could be released soon that’d be really helpful. |
|
I'm going to abandon the major bump plan, as it creates too much resistance to my own ability to spend time working through this. I'm going to split the difference and go back to the original plan of a minor version bump here. Apologies in advance to any semver purists and node 0.12 users out there who will be negatively impacted, but it seems to be the option with the least negative impact. |
https://nodesecurity.io/advisories/566
mapbox/node-pre-gyp#347