Dependency on malicious package

[benmccann]

flatmap-stream is owned by a malicous actor. See here: dominictarr/event-stream#116

This repo depends on flatmap-stream:

chart.js@2.7.3
└─┬ gulp-connect@5.6.1
  └─┬ event-stream@3.3.6 
    └── flatmap-stream@0.1.2 

If you npm install today you'll get flatmap-stream@0.1.2, which I believe to be safe. However, there's no reason why the malicious maintainer cannot publish a new malicious 0.1.3. Adding a package-lock.json file would prevent this.

There's also no way to tell if anyone has been exposed to the malicious flatmap-stream@0.1.1 previously by using Chart.js. It's very likely that our developers have been exposed to this package in the past if they did an npm install at the time where the latest version of flatmap-stream was 0.1.1. Had we been using package-lock.json we would be able to check and take action more easily

I would recommend we adopt package-lock.json and stop using unpinned dependencies. @etimberg @simonbrunel @nagix thoughts?

[simonbrunel]

Adding a package-lock.json file would prevent this.

After reading a few articles about this issue, I don't think that locking dependencies would prevent this case to happen again, except if, each time we push a new version of package-lock.json, we audit the whole tree of dependencies to be sure there is no "unreported" malicious code. This is of course not possible (we don't even audit direct dev dependencies).

Looking at dominictarr/event-stream#116, many repositories with package-lock.json need to be updated and in our case it would have been the same: we explicitly updated our dependencies on November 18th and so we would have pinned the malicious package which appeared on October 5th. And nobody can say for sure if the current version tree is safe.

I would recommend we adopt package-lock.json

We already discussed this a lot, though it was more than a year ago but since I don't think it would have changed this specific case and because we didn't experience any other dependency issues, my position remains the same.

[benmccann]

I was only saying that a package-lock.json would allow us to lock flatmap-stream or event-stream to a known good version so that we protect ourselves from a new version of event-stream being published by the bad actor. This would only protect in this specific case where a package is known to have a malicious maintainer and we want to avoid pulling any new versions of a compromised package. This is no longer necessary since npm has in the meantime removed the event-stream package from its repository so that users can't download any past or future versions of the package.

You're right that a package-lock.json would not protect us from most vulnerabilities. It would help with auditing, however, which I believe has substantial benefit.