ossf / package-manager-best-practices

Collection of security best practices for package managers.
Apache License 2.0
158 stars 19 forks source link

RC npm: install scripts are common attack vectors #15

Closed mlieberman85 closed 1 year ago

mlieberman85 commented 2 years ago

It should probably be noted in the doc that install scripts are often dangerous as they can allow for the running of arbitrary code at installation time. This has been used in the past to try and steal credentials (mostly related to crypto wallets and the like.)

Should suggest in most cases applying: npm --ignore-scripts when possible.

ljharb commented 2 years ago

That suggestion is complicated, because it breaks a lot of valid use cases.

mlieberman85 commented 2 years ago

Well, when possible. There are valid use cases but I also think in general running unknown arbitrary scripts as part of installation should be discouraged, unless there's a way to sandbox it.

I would be also curious to know if there's any effort from npm side to support certain common use cases just in a safer way. Are there any specific use cases that come to mind where you would 100% want to run arbitrary scripts as part of installation?

ljharb commented 2 years ago

I don't think anyone would dispute that install scripts "should be" discouraged.

That, however, is irrelevant - they're currently the only way to accomplish a bunch of valid use cases. As such, until there's a better alternative for these, it would be harmful to recommend ignoring scripts.

Valid use cases include "compile something after install", "download an artifact tailored to the system on install", etc.

lirantal commented 2 years ago

I'm tending towards security by default here and with recommending the use of various methods in which --ignore-scripts comes into play, whether that's via command line, or through the .npmrc config.

laurentsimon commented 1 year ago

So we all agree that scripts are a potential source of attacks.

Given that a lot of things will be breaking if users use the --ignore-scripts, how about the following changes:

  1. In the "Dependency management > Intake" section, we could suggest doing a code review, and explicitly mention the scripts as part of the "code". (most users won't code review, but if they do they will take a look at the script as well)

  2. We can add a note in the same section that there exists a --ignore-scripts for users who want to impose a policy on scripts; warn that because there are genuine use cases it is likely to break; and conclude that for this reason so we do not recommend it unless users verify themselves that none of their dependencies need it.

Wdut?

lirantal commented 1 year ago

Do we have any data as supporting evidence for the statement that "given that a lot of things will be breaking if users use --ignore-scripts" ?

laurentsimon commented 1 year ago

I personally don't; but I suppose listing all the packages that use scripts (possibly with a combination of dependency graph desp.dev) could provide a more scientific answer.

@oliverchang @calebbrown is this data we could pull out from the package analysis project?

ljharb commented 1 year ago

Anything that compiles on install, or downloads assets on install, won't work when ignoring scripts - which includes electron, sass, and a bunch of others.

laurentsimon commented 1 year ago

@lirantal is this convincing enough? Note that we're not ruling out, we're just saying developers may use it but at their own risk. If there's a better way to express it, please let us know.

lirantal commented 1 year ago

Just a small note that this and #20 are very similar so maybe we consolidate this discussion on one of them. @laurentsimon with regards to the above, if we're saying developers may use at their own risk then why even both here with best practices at all? please see my input here

As to the above examples:

  1. Electron isn't something that end users install regularly, is it?
  2. node-sass I accept and am familiar with and I am happy to see it is showing a declining downloads trend
ljharb commented 1 year ago

“Best practices” doesn’t mean “be as paranoid as possible”, it means “have the best balance between security and usability as possible”. Many potential security measures don’t meet that bar - including ignore-scripts.

mlieberman85 commented 1 year ago

So, I would argue it's not paranoia given that a common attack is to use pre/post scripts to run malicious code like stealing crypto wallet keys. Certain other languages don't allow arbitrary pre/post scripts or have built in mechanisms for the package manager to sandbox the installation.

I haven't been around npm much in the past several years, but if we can't suggest ignore scripts, are there suggestions we can make around best practices for sandboxing or avoiding the common attacks?

ljharb commented 1 year ago

"paranoia" doesn't necessarily imply "unjustified" :-)

That's the problem. There is nothing that can be done short of ignoring all scripts, and there are no alternatives for package authors to avoid using scripts for the many relevant use cases.

Until npm provides alternatives - or more granular protection mechanisms - the status quo (virtually nobody ignores scripts, and those who do tend to run into problems) remains unchanged.

jeffmendoza commented 1 year ago

Closing with same reasoning as #20

naugtur commented 1 year ago

@ljharb issues with --ignore-scripts have been solved by @lavamoat/allow-scripts

It covers:

No functionality depending on install scripts is broken.

There is no other way to defend against well crafted malicious packages using this technique. And they're becoming more and more common.

Full talk on this topic exactly: https://m.youtube.com/watch?v=y2p1e7UmGYY

ljharb commented 1 year ago

Replied in #20.