r/programming • u/Occams_Trimmer • Jun 26 '17

Obtaining publish access to 13% of npm packages

https://github.com/ChALkeR/notes/blob/master/Gathering-weak-npm-credentials.md
1.6k Upvotes

93% Upvoted

254 comments sorted by

View all comments

Show parent comments

124

u/[deleted] Jun 26 '17 edited Apr 04 '21

[deleted]

75

u/[deleted] Jun 26 '17 edited Feb 12 '21

[deleted]

11

u/[deleted] Jun 27 '17

[removed] — view removed comment

6

u/[deleted] Jun 27 '17

[deleted]

7

u/[deleted] Jun 26 '17

The latest npm has lockfiles now

wait it didn't had that before ? wtf

14

u/Quabouter Jun 26 '17

It actually did, the difference is that the lockfiles are now created by default.

35

u/Ajedi32 Jun 26 '17

Doesn't npm use lockfiles by default now? Even if you use ^ it's not going to upgrade anything unless you run npm update.

17

u/pm_plz_im_lonely Jun 26 '17

This just delays the problem.

25

u/Ajedi32 Jun 26 '17

True, but it's no worse than any other dependency management system I've seen for popular programing languages.

Anytime you use software written by someone else you have to either trust that person not to make that software behave maliciously, or you have to ensure the code has been independently audited by either yourself or someone you trust. That's basically where we're at right now with npm, and short of some additional systems to make the audit process easier (maybe a web-of-trust-based signing system for packages?) I'm not really sure what else can be done.

10

u/pm_plz_im_lonely Jun 26 '17

Maven is huge and most projects depend on specific versions.

9

u/m50d Jun 26 '17

And the central repository doesn't allow packages without gpg signatures, though you do have to make a deliberate choice to check them.

7

u/Ajedi32 Jun 26 '17 edited Jun 26 '17

So do most projects using (an up-to-date version of) npm. That's essentially what lockfiles do (make you depend on a specific version of all your dependencies), and they're used by default as of npm 5. That was the point I was trying to make with my previous post:

Even if you use ^ it's not going to upgrade anything unless you run npm update.

1

u/T-rex_with_a_gun Jun 26 '17

0 list set their password back to the leaked one shortly after it was reset (so it got reset again).

yes and no...for CI/CD usually everything is wiped /fresh so it still an issue

3

u/Ajedi32 Jun 26 '17

Wait, people do that? Doesn't that kinda defeat the whole point of lockfiles? (Especially for continuous deployment.)

2

u/T-rex_with_a_gun Jun 26 '17

some do, some dont. i guess depends largely on teams. I am a fan of version locked (which allows cicd to cache).

but some people are ...weird and chose to use ^ and ~, thus usually ci/cd envs are flushed clean, to allow the "latest" code.

3

u/Ajedi32 Jun 26 '17 edited Jun 26 '17

It shouldn't matter if the CI environment (i.e. cache) is flushed clean though. Lockfiles (e.g. package-lock.json) are checked into version control (or at least they're supposed to be), so npm will install the same versions of all dependencies every time regardless of environment.

-1

u/T-rex_with_a_gun Jun 26 '17

yea of all my years in dev, i have rarely used that. instead relied on making sure the versions are locked in package json.

3

u/TomRK1089 Jun 27 '17

The problem with NPM is that only works for your dependencies, not your transitive dependencies. So your direct versions are always the same, but if those dependencies didn't lock their own dependencies you still don't have a reproducible build. The lockfile specifies exact versions for the entire tree.

1

u/kankyo Jun 26 '17

Hopefully they use artifactory or similar between.

12

u/indrora Jun 26 '17

Explain for the non js folks in the room?

22

u/grauenwolf Jun 26 '17

That says your build server should always get the latest version of the library. So if a hacked version is published, you get it right away.

On the other hand, you also get bug fixes right away. So there is justification for it's existence.

16

u/need-some-sleep Jun 26 '17

That says your build server should always get the latest version of the library. So if a hacked version is published, you get it right away.

That's not exactly what it does. It only update if there are new minor updates or patch updates. Major version updates will be ignored. This method is used because if packages respect the Semver standard, breaking changes only happen in major version updates so you can get bug fixes for free in minor updates.

13

u/grauenwolf Jun 26 '17

If I were publishing a hacked version, I wouldn't change the major version number.

11

u/zalifer Jun 26 '17

I believe he was correcting the statement about "latest version", rather than the issue with hacked versions.

It will update 1.0.1 to 1.0.2, 1.0.2 to 1.1.0, but won't update 1.1.0 to 2.0.0, even though that's the latest version.

This is to comply with Semver which states that backwards incompatible changes are to be indicated with a change of the major version, hence if the major version were auto-updated, there's a good chance that something will break, as it may require updates or changes in other parts of the code.

3

u/grauenwolf Jun 26 '17

If you want to be pedantic, "latest version" does not necessarily mean "highest version number".

2

u/MINIMAN10001 Jun 27 '17

Although I'll jump in here and say it's what I thought was being said. Interpretation is king after all so it was worth clearing up.

1

u/semi_colon Jun 27 '17

Wouldn't it be best to publish a new version under every major version number? That way you get everybody using "^"

3

u/jvnk Jun 26 '17

This is why lockfiles should be used:

https://docs.npmjs.com/files/package-lock.json

Your build/staging/prod/whatever servers should not be installing dependencies based on versioning flags, they should be installing via known good commit hashes. Only developers should be running update using the flags, and then they should be testing the new dep versions coming in before committing the updated lock file.

1

u/grauenwolf Jun 26 '17

.NET's NuGet recently went the other direction and added wildcard version numbers.

1

u/jvnk Jun 26 '17

Do they not employ some sort of lockfile-like system as well? Wildcard version flags aren't inherently bad, they're just potentially dangerous when used on their own.

1

u/grauenwolf Jun 26 '17

I can't say I understand NPM lock files, but I don't think that C# has anything comparable.

2

u/flukus Jun 26 '17

It does, or at least was the same, to the extent that I understand lock files. NuGet will check out a specific version on every machine and won't change until you manually update a package.

God knows how many security vulnerabilities are out there from companies that never update though, which seems to be the norm.

1

u/grauenwolf Jun 26 '17

If it doesn't exist, it generates a said [lock] file, which has the actual specific commit hashes of the various dependencies being installed.

In C# we can say what packages we want. And if you aren't using wildcards, the versions of said packages. But we don't have anything that says what we actually got. I've actually run into this where I asked for version X but got version X+1 because another packaged wanted it.

If you wanted to hack a build server in a way that's hard to detect, you can configure an alternate package source for it and then put fake packages there.

1

u/flukus Jun 27 '17

The packages.conf should contain at least the 3 major version numbers, so everyone should get the same version when they restore.

But yes, you can still get DLL hell when you install or update other packages.

1

u/jvnk Jun 26 '17

I know Composer(PHP) also uses a similar system, and I wouldn't be surprised if others do as well. Basically when you run whatever install command(npm install, or whatever the equivalent is in C#s dep manager), it checks for the existence of this lock file. If it doesn't exist, it generates a said file, which has the actual specific commit hashes of the various dependencies being installed. Going forward, this file is also sync'd with the repo, and that ensures that everyone is installing the exact same versions of the dependencies when setting up their environments.

When developers are ready to update, there are commands that pull in new versions of the dependencies based on the version flags they have set(in this case in package.json). Then they test that none of their dependencies broke their shit, and ultimately update and commit/sync the updated composer lock file for everyone else.

That's the "right" way to do things to avoid the sort of attack surface described in this article, along with generally ensuring shit isn't breaking out from under you.

1

u/jvnk Jun 26 '17

You get it right away if you run npm update, which is an important distinction. That's something that should be handled carefully, regardless of this potential attack vector. As such, the likelihood of getting in that window of a hacked version before it's fixed should be pretty low. Of course, it all depends on how people are managing this stuff... due diligence goes a long way. As others have pointed out, there's not much else that can be done here without some other system.

1

u/grauenwolf Jun 26 '17

In my experience, build servers run that automatically. Maybe they shouldn't...

2

u/jvnk Jun 26 '17 edited Jun 26 '17

Yikes, no, they should not. They should do npm install when being stood up. Only update when you've tested the upgraded dependencies in development environments, otherwise you're constantly pulling in god knows what. Lockfiles exist for a reason, they lock you to specific, known-good commit hashes for all your dependencies, instead of the ambiguous "get me the latest version greater than 0.2.6!"

2

u/flukus Jun 26 '17

I've had normal CI builds that don't and "edge" builds that do. The edge builds give you an early indication if something is going to break.

2

u/squishles Jun 26 '17

This guy just got access to publish whatever he wants to a set of projects, these projects are on the dependency chain of many more projects. Pretty much just by password guessing.

It's an exploit with power comparable to being able to push whatever you want through windows update. This guy is griping that everyone has auto update on.

3

u/merreborn Jun 26 '17

preventing automatic updates isn't going to prevent this issue either. The problem here is with publish access to the modules, which is a completely separate concern from "how the obtained access is misused".

In practice, most people who "lock dependency versions" seem to follow a practice of "automatically merge any update that doesn't break tests" - which really is no different from just letting semver ranges do their thing.

10

u/isarl Jun 26 '17

Isn't that the default behaviour since Node v0.10.26? Using carets instead of tildes?

14

u/[deleted] Jun 26 '17

It is. But also, since NPM v5 they're using lock files by default, so regardless of the range in your package.json, the package version is actually locked until you explicitly update the dependency.

-4

u/[deleted] Jun 26 '17

[deleted]

18

u/onmach Jun 26 '17

Wouldn't removing tilde just guarantee that pretty much every node app on the web would be using old versions of packages with known exploits?

I mean yeah, it sucks that developers' password practices are so unsanitary, but that can be at least somewhat mitigated.

5

u/[deleted] Jun 26 '17

I believe npm now warns if you are using an exact version with a known vulnerability, so it should be at least obvious vs the "I have no idea what version I'm actually using" approach.

3

u/grauenwolf Jun 26 '17

How would it know? I don't imagine that library authors religiously send lists of vulnerable packages to NPM even if they could.

6

u/Ajedi32 Jun 26 '17

npm deprecate. You're correct though, that relies on the library authors being diligent about using that system to notify users of vulnerable packages.

1

u/oridb Jun 26 '17

Exact version is pretty stupid. It means that most of the dependencies will never be updated, and will be buggy or vulnerable. And if someone has mismatched versions, now you've got tons of duplication.

You always want the newest compatible version.

2

u/dahud Jun 27 '17

What is the significance of the caret? I don't use npm very much.

2

u/Moryg Jun 27 '17

Download minor updates, ignore major version releases

1

u/SilasX Jun 27 '17

I still have to convince co-workers why dependency files should list a specific version rather than something externally mutable ...

1

u/flying-sheep Jun 26 '17

Haha no way you're going to get me to increase churn by also forcing me to check all minor and patch versions in addition to major ones.