[BUG] NPM allows insecure code execution by configuration file

[rotem-cider]

Is there an existing issue for this?

• I have searched the existing issues

This issue exists in the latest npm version

• I am using the latest npm

Current Behavior

Intro

NPM is a package manager used in our CI/CD environments, We are using it to download and install all our different dependencies inside our building phase for production.

In order to use it safely, we use some precautions, We try to minimize as much as possible its exposure to secrets but still sometimes we have no choice but to add them.

Reading the documentation here and best practices using the cli around the internet it is best to run in ci environments with npm ci --ignore-scripts which should not run any scripts, additionally, we saw that npm sees bypassing this command as a high severity

Researching this subject we found out that a developer or malicious actor with access to the codebase can in fact force npm to run scripts although we configured it explicitly to ignore scripts.
Talking with the bounty team the response was that this is not eligible for bounty and is not considered a risk as this is the intended behavior of npm.

We agree that this is a design issue, but nevertheless, this can be used as an attack vector by actors, we want to raise the issue here and understand what mitigations can we have in the meantime, and if this design will be changed in the near future?

The Issue

When adding a .npmrc file to our repo, npm will pick up the file and read its configuration. This means that any command using npm will automatically pick up the configuration file.

It is possible to inject a configuration that will then execute code in the installation context and attempt to attack the installation environment by two vectors:

1. Adding git=${PWD}/rce.sh to .npmrc and adding an installation from git to the package.json, such as "dependencies": { "ini":"git://github.com./npm/ini.git#v2.0.0" } will execute the rce.sh script when running npm install version 7,8
2. Adding onload-script=${PWD}/rce to .npmrc will invoke the rce.js script in the library when running ANY npm version 6 command

Expected Behavior

As we are not expecting to run any scripts when running with "--ignore-scripts" this is can and will affect our devops installations if a malicious actor gains access to parts of our codebase.

I understand that this was not the intended behavior of the flag,
but it became the only security measure in use and implies that there will be no unintended scripts running

Steps To Reproduce

Using npm version 7,8

1. npm init
2. npm i --save "git://github.com./npm/ini.git#v2.0.0"
3. echo "git=${PWD}/test.sh" > .npmrc
4. echo 'echo "HACKED!!!" > poc.txt && git $@' > test.sh
5. rm -rf node_modules

Attack phase

1. npm ci --ignore-scripts
2. cat poc.txt

Using npm version 6

1. npm init
2. echo "onload-script=${PWD}/rce" > .npmrc
3. echo "console.log('Hacked") > rce.js

Attack phase

1. npm ci --ignore-scripts

Environment

• npm: 8.1.4
• Node: v14.15.5
• OS: OSX
• platform: Macbook Pro
• npm config:

; "user" config from /Users/rotembar/.npmrc
registry = "https://registry.npmjs.org/"

; "project" config from fuzz/npm-lion/.npmrc
git = "npm-lion/test.sh" 

[ljharb]

How is this any different than running npm run foo and defining the "foo" script to run insecure code? If someone has control of the local filesystem, they can always run whatever code they want (albeit with constrained permissions)

[ntindle]

I would argue that to be true if it wasn’t a CI command. Even if this isn’t changed, it’s a good thing to document.

[rotem-cider]

Hey @ljharb , this is different in the case where the developer does not control the ci environment.

in a scenario where I know builds will run with “npm ci —ignore-scripts” (so all the normal scripts won’t run) then the developer should not be able to force the build scripts to execute his code.

because of this scenario the developer can always bypass with this method and execute his own code.

if for instance in the pipeline there was a npm run foo, then the developer would be able to change the foo script and run it. Then best practice would be to run it in a subshell without any env variables which may leak.

worst in npm6 any command can be hijacked, so even “npm publish” that usually has write permissions will execute the developers code

[aviadhahami]

I agree with @rotem-cider.
While I'm aware this is a feature, a user who's unfamiliar with .npmrc files may run npm x and the shell will be executed.

The major difference between having a malicious script foo and this case, is that foo will run iff one explicitly runs the foo script(npm run foo), while here even a basic npm ci will result in RCE (regardless of --ignore-scripts specification)
This is unexpected behavior for the untrained eye.

while I know that scripts can be called using the pre/post hooks, they can be disabled using --ignore-scripts. In the custom binary case the flags are not respected

[ntindle]

https://docs.npmjs.com/cli/v8/commands/npm-ci#ignore-scripts

The way I read this is that is lifecycle script behavior is undefined according to the docs.

[DanielRuf]

I found this issue while reading the article at https://snyk.io/blog/visibly-invisible-malicious-node-js-packages-when-configuration-niche-meets-invisible-characters/

That reminds me of the log4j CVE labeled as RCE which was an ACE and was about something similar (configuration file changes or code changes could allow arbitrary code execution).

I agree with @ljharb.

if for instance in the pipeline there was a npm run foo, then the developer would be able to change the foo script and run it

How is this different from simply changing the code in your own fork or with GitHub Codespaces?

There are some security recommendations regarding pipelines.

Also on GitHub Actions you can define some further settings (who is allowed to run the actions, which actions are allowed to run, who can run the actions for PRs, ...).

Further information:
https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks
https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections
https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#good-practices-for-mitigating-script-injection-attacks
https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#modifying-the-contents-of-a-repository

https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-nodejs-or-python

I didn't verify this but shouldn't the highest verbosity level of npm make such tricks visible?

[rotem-cider]

@DanielRuf you have some valid points but are reliant on the fact that all companies work with strict configurations.

First about log4j, it is a full easy to create and exploit remote code execution which affected thousands of companies by default configuration. I think the comparison was wrong, also that this vulnerability is not critical like log4j but very important.

About the ability to code execute in the pipeline, as the pipeline is a very sensitive area which directly controls what will be in our final production build but also has Access to many secrets a company should protect it with proper security measures

adding the security measures you mentioned In GitHub is super important, especially to make sure no untrusted developer can execute code in secret aware areas in the ci.

Unfortunately these measures are not always used, not enough and companies have many different ci systems which have different security measures (jenkins, circleci, codefresh, …)

as for the GitHub security measures:

1. As this bypass happens inside the GitHub action, the measure you mentioned about when running GitHub actions is not relevant
2. Not running for first time contributors may help, but bypassing it is super easy, just request to fix a comma or dot in the repo, and you become a validated commuter
3. Read permissions give you a lot, especially when your goal is to read secrets and use them externally

When separating properly the different actions and giving them proper permissions we can avoid such issues but when we have a wildcard that allows users to submit a file and wait for shell our job as security engineers gets much more complicated

you can read more implications about different perspectives of attacking the ci in Omer’s excellent blog post here - https://www.cidersecurity.io/blog/research/ppe-poisoned-pipeline-execution/

[ljharb]

@rotem-cider if you use a dependency's code, you are trusting it. You run your dependencies' code in dev, CI, and often prod. Someone who wants to use this as an attack vector can also just write code to do it, instead of relying on npm to do it - and npm typically never runs in prod, making it safer than "node itself" in this regard.

[DanielRuf]

@rotem-cider regarding log4j, I do not mean the original CVE but this one: CVE-2021-44832 (ACE caused by manipulated configuration, where attackers need access to your sourcecode)

Regarding the other points: you can always make it more strict, please see my links and screenshots.

Still, someone who has access to the sourcecode can do anything. That has nothing to do with npm and any other program with a config and even env variables can be theoretically abused. This is by design and technically no real vulnerability.

[lirantal]

I think the nuance here, at least with regards to Aviad's article on employing both trojan source and configuration overwriting is that this doesn't have to be a code/dependency issue at all. If a collaborator is able to somehow trick you into making a change to the project's .yarnrc / .npmrc and also to somehow sneak in the malicious code as part of the repository's source code then they put the project maintainers at risk.

I do agree that at this is however not much of a higher elevated risk than anyone just proposing a pull request with malicious code/intent behind it. That said, the interesting use of trojan source attacks (invisible characters, etc) for high-impact insecure defaults is quite interesting. At the very least, this reminds me my research about lockfile insecurities which will mostly go unnoticed to anyone.

So, to be more practical, I'd ask not whether we want to spend more time to convince each other if it's a vulnerability or not, but what actions can we drive forward to advance security for end-users. Are there changes we can drive to package managers with regards to secure defaults?

[ljharb]

That still requires us to agree on what defaults are more secure. What do you suggest here?