cat posts/2018-11-27-the-malicious-dependency-in-our-build.md
The malicious dependency in our build
“I don’t know what to say.” That’s the title of a GitHub issue on event-stream, an npm package with something like two million downloads a week, and it was the first thing I read yesterday morning, a little before eight. By the end of it I’d stopped drinking my coffee.
If you missed it, here’s the story as it’s been pieced together in that thread over the last week: the original author of event-stream had stopped using it years ago. In September someone he didn’t know offered to take over maintenance, made a few reasonable-looking commits, and was given publish rights. They then added a new dependency, flatmap-stream, published version 3.3.6 of event-stream that pulled it in, and a few weeks later a version of flatmap-stream appeared containing obfuscated, encrypted code. A developer chasing down a deprecation warning in his own project traced it back to that code and opened the issue on the 20th. The encrypted payload turned out to be aimed at exactly one target, the Copay bitcoin wallet. It only decrypted when it was being built into an app whose package description matched Copay’s, and then it tried to steal wallet keys. BitPay has said Copay versions 5.0.2 through 5.1.0 shipped with it. npm took the malicious package down yesterday.
So the first question at 8 a.m. was the obvious one. Do we have it?
Looking
We have a dozen-odd repos with a package.json in them: the main web app, a couple of internal tools, the marketing site, some Lambda functions, a CI tooling repo. An engineer on my team and I split them and ran npm ls event-stream flatmap-stream in each, which walks the installed tree and shows the path to any match. That only tells you what’s installed in your checkout, though, so we also grepped every committed package-lock.json for the two names, since the lockfile is what CI actually installs from.
Results, by about 8:40. event-stream showed up in four repos, all of them as a transitive dependency of a file-watching dev tool, three levels down. Three of those had it locked at 3.3.4, from before the takeover, and were clean. The fourth, the CI tooling repo, had 3.3.6 and flatmap-stream 0.1.1 in its lockfile. Someone had run npm update there in October to fix an unrelated warning, it pulled in the latest of everything allowed by our version ranges, the diff was a few hundred lines of lockfile, and the PR was approved by someone (me, it turns out) who scrolled past the lockfile. After left-pad we’d agreed that a reviewer would at least skim what was new in a lockfile diff (I wrote about it in 2016). I was the reviewer, and I didn’t.
Was it bad?
Honestly, we were lucky, and I want to be precise about why. By the time we looked, people in the issue thread had already decrypted and published an analysis of the payload, and it did nothing at all unless it was running inside Copay’s build. We don’t build a bitcoin wallet. So the code was on our CI machines, it ran its decryption attempt, it failed, and it did nothing else.
But I only know that because someone else did the analysis first. If we’d found it in our lockfile on the 20th, before anyone had decrypted it, we’d have been looking at an encrypted blob running in our CI environment, which has deploy credentials, and we’d have had to treat it as if it had taken everything. That’s the version of this I spent yesterday afternoon thinking about. Rotating every secret CI can see is about a day and a half of work for us, and it’s the kind of work you want to have a list for before you need it. We now have the list.
We didn’t rotate, in the end. We did rebuild the CI images from scratch and re-run the dependency install with the fixed lockfile, and we checked egress logs from CI for the window it was installed, for anything that wasn’t the hosts we expect. There wasn’t anything.
What we pinned
Most of what we changed is about not being surprised next time. Every repo now has a lockfile committed (the marketing site didn’t, because nobody had counted it as “our code” when we made the rule in 2016), and CI installs with npm ci, which fails if the lockfile and package.json disagree instead of quietly resolving something new. The dev tool that dragged event-stream in was pinned to a version that doesn’t, and we’ve added npm audit to CI. It wouldn’t have caught this before the advisory went out, but it does catch the next one the day it’s published.
The bigger change is that a PR that adds a new package to a lockfile, anywhere in the tree, now gets a short generated comment listing the new package names and who published them. That’s cheap to build. Whether anyone reads it is another matter, and I’m aware I’m the person who approved the October diff.
The part I don’t have an answer for
Our main web app’s lockfile has a little over 1,100 packages in it. We chose about 40 of those directly. The rest are what those 40 needed, and what they needed, and so on. I couldn’t name the maintainers of more than a handful, and as of this week I know that any one of them could, for perfectly understandable reasons, hand the keys to a stranger who asks politely. The original maintainer here did nothing wrong by any normal open source standard. He wasn’t being paid, he didn’t use the code anymore, and someone offered to help.
Pinning protects us from new versions. It doesn’t help when the version we pinned is already the bad one. Reviewing 1,100 packages isn’t possible, and neither is writing them ourselves. Paying maintainers would help, and some companies are starting to, but it’s not going to reach a three-levels-down dependency of a file watcher. I’d like to hear what other teams are doing, because what we have right now is a lockfile, a bot comment and a lot of hope.