Skip to content
This repository has been archived by the owner. It is now read-only.

ESLint-scope 3.7.2 has been hacked #21202

Open
pronebird opened this Issue Jul 12, 2018 · 24 comments

Comments

Projects
None yet
@pronebird
Copy link

pronebird commented Jul 12, 2018

Please see the following issue:
eslint/eslint-scope#39

eslint-scope 3.7.2 has been published an hour ago which is a hacked version that steals the NPM accounts or something.

Please pull the version 3.7.2 from the npm and freeze the account so this does not get propagated.

As a matter of fact, there is no release tag for 3.7.2 on Github, so I think it would be great to consider double checking with Github repository before publishing any code.

This would at least limit the possibility of uploading the malicious code to NPM without having Github credentials to tag the release/version.

Additionally I believe it's possible to check if the release was signed and somehow enforce all tagged commits to be signed. I think Github returns such information via their API, at least you can see the verified commits via Github's web interface so there must be a way. Developer may be able to opt-in for this extra security via some .rc file stored in Git repo.

@akx

This comment has been minimized.

Copy link

akx commented Jul 12, 2018

3.7.2 seems to have been unpublished now. Would love to hear what actually happened.

@pronebird

This comment has been minimized.

Copy link
Author

pronebird commented Jul 12, 2018

@akx shortly, the npm credentials have been stolen and the malicious release has been made directly to npm (not GitHub).

The malicious code ran upon npm install and attempted to transfer the npm authentication token from .npmrc to remote server.

Surprisingly, the tampered code only contained a bootstrap script that downloaded and executed the main script (with eval) from pastebin and the pastebin script had a syntax error in it so that's how it revealed itself.

All of what happened is something to think about, why didn't npm spot the calls to eval and reject the update. I know many packers use eval as well but come on, it's not 1999 again and there are simple measures and protocols that can be implemented in order to strengthen the security of npm ecosystem.

@branneman

This comment has been minimized.

Copy link

branneman commented Jul 12, 2018

The first question that came to mind after seeing this issue is: why is there a difference between the published code on npm, and the code on github? I know why, because when you run npm publish, you publish from a local directory.

I think npm can increase security by not letting people publish code from their local directories but only download code from a public repo like github’s (ala server-to-server). It would of course only be possible for public modules/repositories. That way all published code always has a history.

It increases security because to publish malicious code you need to:

  1. steal both npm credentials and github credentials
  2. commit and push malicious code to github
  3. pass security a project's build pipeline (and possible security scans)

I'm probably not the first with this idea, but has it been considered by the npm team yet?

I can also imagine that the world would be a safer place if npm publish would require an npm account with 2FA enabled.

@pronebird

This comment has been minimized.

Copy link
Author

pronebird commented Jul 12, 2018

@branneman that's true, but some code is transpiled upon release so this has an impact of forcing developers to store all their stuff in git or put the strain on npm servers to pull the code and run all build procedures before publishing. Anyway, there must be a way. 👍

@jpdriver

This comment has been minimized.

Copy link

jpdriver commented Jul 12, 2018

Curious thing, all tampered files had the modification date set to Oct 26, 1985. So I guess now we know the birthday of whoever who did this...

fyi @pronebird all files showing Oct 26, 1985 (a Back to the Future reference joke) is actually intented behavior for NPM (see #20439 (comment)) and not related to this hack.

@nakedible-p

This comment has been minimized.

Copy link

nakedible-p commented Jul 12, 2018

The actual solution would be https://reproducible-builds.org/

Fixating on one build environment is not a good idea in the grand scheme of things.

@pronebird

This comment has been minimized.

Copy link
Author

pronebird commented Jul 12, 2018

@jpdriver Thanks, I didn't know this. I mostly use yarn, so 1985 came as a surprise. 👍 (I removed the mention of this everywhere)

@co3k

This comment has been minimized.

Copy link

co3k commented Jul 12, 2018

Has already the attacker got tokens by this attack? If so, will other packages be hacked again?
Do you have a plan to revoke all npm users' token or to stop publishing any packages?

@EdwardDrapkin

This comment has been minimized.

Copy link

EdwardDrapkin commented Jul 12, 2018

It seems like the reasonable thing to do would be:

  1. Temporarily halt all package publications
  2. Search the entire registry for references to this virus and unpublish any infected packages
  3. Globally revoke all authentication tokens
  4. Re-enable package publications
@ghost

This comment has been minimized.

Copy link

ghost commented Jul 12, 2018

The key is revoking all npmrc tokens globally before the attacker takes action.

If this isn't done soon enough, the only way to completely prevent the damage will be the manual audit of the almost the npm registry, and that is borderline impossible.

(so I'm really hoping npm revokes everything soon enough, otherwise most people will just have to stop using it)

@ghost ghost referenced this issue Jul 12, 2018

Closed

Virus in eslint-scope? #39

@paulirwin

This comment has been minimized.

Copy link

paulirwin commented Jul 12, 2018

@EdwardDrapkin Agree with all of your points, but I'd go a step further with number 2: in addition to searching the registry, unpublish all packages published since the affected version was published (and even then, it's not clear yet if the eslint-scope infection was a result of a further upstream infection). The virus could have modified itself (or have been manually modified, or use a different version entirely) so as to be unrecognizable from the original. There's nothing to say that the pastebin code from this incident is the same as what would be infected in other packages of authors with their credentials compromised. The credentials are being logged to web counters, which to use the credentials they would have to be monitored from an outside source, so there's nothing stopping that outside source from publishing a different script entirely.

Edit: upon further thought, I realize that my suggestion should only be done if it is determined that a significant amount of packages are affected after halting all package publications and revoking all tokens. It's possible that could effectively resolve this specific incident. However this should at least be a "shot across the bow" that a single token being compromised can have disastrous ripple effects across the ecosystem.

@forrestab

This comment has been minimized.

Copy link

forrestab commented Jul 12, 2018

Everyone, I am not sure where else to put this and the eslint-scope issue has been locked, but is there anyway to search the npm cache to make sure this version is not cached?

I am on windows if it helps or doesn't.

@OliverUv

This comment has been minimized.

Copy link

OliverUv commented Jul 12, 2018

Surprisingly, the tampered code only contained a bootstrap script that downloaded and executed the main script (with eval) from pastebin and the pastebin script had a syntax error in it so that's how it revealed itself.

The script seems to have run successfully on one user's Windows system. The "problem" in the script seems to have been that it would only run correctly if the full source code was fetched in the first chunk - the malware author forgot to ensure all chunks had been fetched. I gleaned this from the original eslint-scope issue.

So it is reasonable to expect many users' auth tokens to have been stolen.

@sr229

This comment has been minimized.

Copy link

sr229 commented Jul 12, 2018

I +1 the global unpublishing of all packages and revoking all tokens until this is resolved. This does NOT look good.

@cphoover

This comment has been minimized.

Copy link

cphoover commented Jul 12, 2018

@branneman and others suggesting npm integration into git,

Not every npm package uses git, and certainly not everyone uses github. Marrying the two concepts doesn't make sense to me.

I think the best solution is faster communication and awareness of exploited packages.

Some brainstorming ideas:

  • Maybe integrate the npm ecosystem into something like node security project. Warnings should be shown inside of NPM cli when a vulnerable package is found in the dep tree.
  • Easy flagging of infected code to notify maintainers.
  • Have stricter publish regimens for foundational (heavily depended on) dependencies. Maybe enforce manual audits with two factor authentication. Realistically dependencies that have thousands and thousands of dependents should not be modified so frequently, or they must go through a manual audit step.
  • Perhaps have a badge awarded to releases that have been manually audited and approved with a 2 factor verification.
    ... I could go on
@gedgaudasnikita

This comment has been minimized.

Copy link

gedgaudasnikita commented Jul 12, 2018

Just forbid all packages containing a string "eval", or make them require manual reviewal. But my advise would be just ban them for good

@ThiefMaster

This comment has been minimized.

Copy link

ThiefMaster commented Jul 12, 2018

@gedgaudasnikita how would that help? then they write the code to a file, and require() it.. or download a compiled binary instead of using JS for their malware.

btw, why do you have two accounts? @nikita-gedgaudas-ht / @gedgaudasnikita - AFAIK the GitHub ToS only allow one account per human

@cphoover

This comment has been minimized.

Copy link

cphoover commented Jul 12, 2018

@gedgaudasnikita yea but what about using Function constructor syntax::

new Function('console.log(1 + 1)')()

I feel like you are addressing the symptom by banning eval not the root problem.

@sr229

This comment has been minimized.

Copy link

sr229 commented Jul 12, 2018

@gedgaudasnikita That won't work. Eval is needed for a lot of things, and banning it won't do. We need package signing - which NPM actively rejected, but I feel like this has a use right now judging from the current events right now

@OliverUv

This comment has been minimized.

Copy link

OliverUv commented Jul 12, 2018

@gedgaudasnikita

Just forbid all packages containing a string "eval", or make them require manual reviewal. But my advise would be just ban them for good

This does not work well with some JS perf optimization patterns. See the monomorphization that improved webpack's speed by ~80% between version 3 and 4.

@OliverUv

This comment has been minimized.

Copy link

OliverUv commented Jul 12, 2018

Probably the only acceptable long-term solution is to allow and encourage package signing, with the ability to use a second factor (such as a hardware key) for the signing. Whoever stole the eslint-scope creds could also have stolen signing creds, if they were not using 2 factor auth.

It will take a long time for the majority of package maintainers to reach this level of security awareness, but it's a project that must be started.

@sr229

This comment has been minimized.

Copy link

sr229 commented Jul 12, 2018

@OliverUv +1 on that but I don't know why NPM rejected this - it was a majorly a good security practice

@zkat

This comment has been minimized.

Copy link
Member

zkat commented Jul 12, 2018

We've updated our status page about this incident. I'm going to lock this thread because it's likely to draw a lot of traffic and I think actual information will be buried. Please keep an eye on the status page, our social media, npm.comminuty announcements, or this thread for any particularly big things.

While you're here, a reminder that this issue tracker is closing in favor of npm.community, so please direct issues and such to that space. This issue tracker will be closing very soon.

@npm npm locked as too heated and limited conversation to collaborators Jul 12, 2018

@zkat

This comment has been minimized.

Copy link
Member

zkat commented Jul 12, 2018

(update: apologies, my original post included a link to this issue instead of the actual status page. The link has been updated)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.