Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] npx@8 regression: command injection / impossible to pass a bactick as argument #4873

Closed
2 tasks done
lydell opened this issue May 9, 2022 · 3 comments · Fixed by npm/run-script#78
Closed
2 tasks done
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 8.x work is associated with a specific npm 8 release

Comments

@lydell
Copy link

lydell commented May 9, 2022

Is there an existing issue for this?

  • I have searched the existing issues

This is a duplicate of #3306, which was closed by mistake.

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Let’s say you have a create-blog-post CLI installed globally. You run it like so:

create-blog-post --title 'Cool `ls` tricks'

One day you install create-blog-post locally instead. Then how do you run it? Well, you could just slap npx at the start, right? Wrong! The following does not do what you expect:

npx create-blog-post --title 'Cool `ls` tricks'

Let me show why. I’m using node -p 'process.argv[2]' -- instead of create-blog-post to show that the implementation of that tool wouldn’t matter:

❯ node -p 'process.argv[2]' -- --title 'Cool `ls` tricks'
Cool `ls` tricks

With npx in front:

❯ npx node -p 'process.argv[2]' -- --title 'Cool `ls` tricks'
Cool LICENSE
README.md
lib
map.js
node_modules
package-lock.json
package.json
test tricks

Oops! The argument was treated as shell script, executed ls and put the result in my string (backticks means command interpolation)!

Expected Behavior

npx@6 got it right:

❯ npx --version
6.14.12

❯ npx node -p 'process.argv[2]' -- --title 'Cool `ls` tricks'
Cool `ls` tricks

The worst thing is that I don’t even know how to workaround this issue in npx@8. Trying to add backslashes does not help. I just can’t figure out a way to pass literal backticks as an argument.

Steps To Reproduce

  1. macOS or Linux (Windows have different issues)
  2. With this config...
  3. Run npx node -p 'process.argv[1]' '`' (tested in sh, bash, zsh, fish)
  4. See error: sh: -c: line 0: unexpected EOF while looking for matching ``'

Environment

  • npm: 8.9.0
  • Node.js: v18.1.0
  • OS Name: Linux
  • npm config:
; node bin location = /usr/local/bin/node
; node version = v18.1.0
; npm local prefix = /
; npm version = 8.9.0
; cwd = /
; HOME = /root
; Run `npm config ls -l` to show all defaults.
❯ docker run --rm -it node:18 bash
root@a78d74c8a6b5:/# npm i -g npm

changed 15 packages, and audited 202 packages in 6s

11 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
root@a78d74c8a6b5:/# npm -v
8.9.0
root@a78d74c8a6b5:/# node -v
v18.1.0
root@a78d74c8a6b5:/# uname -a
Linux a78d74c8a6b5 5.10.104-linuxkit #1 SMP Wed Mar 9 19:05:23 UTC 2022 x86_64 GNU/Linux
root@a78d74c8a6b5:/# npm config ls
; node bin location = /usr/local/bin/node
; node version = v18.1.0
; npm local prefix = /
; npm version = 8.9.0
; cwd = /
; HOME = /root
; Run `npm config ls -l` to show all defaults.
root@a78d74c8a6b5:/# npx -v
8.9.0
root@a78d74c8a6b5:/# npx node -p 'process.argv[2]' -- --title 'Cool `ls` tricks'
Cool bin
boot
dev
etc
home
lib
lib64
media
mnt
opt
proc
root
run
sbin
srv
sys
tmp
usr
var tricks
root@a78d74c8a6b5:/# node -p 'process.argv[2]' -- --title 'Cool `ls` tricks'
Cool `ls` tricks
root@a78d74c8a6b5:/#
@lydell lydell added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels May 9, 2022
@nlf
Copy link
Contributor

nlf commented May 9, 2022

thanks for opening a fresh issue! i had it on my todo list for today to reopen the old one but this is great. thanks again

@nlf nlf self-assigned this May 9, 2022
@nlf nlf added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels May 9, 2022
@juristr
Copy link

juristr commented May 25, 2022

I have a similar, kinda related issue where the npx command would not respect escaping of special symbols, like $ for instance.

If you have a node script and invoke it like node myscript --somearg=$foo then $foo would be interpreted as an environment variable. And I can work around this by escaping it like node myscript --somearg=\$foo which works just fine.

If however I invoke it with npx node myscript --somearg=\$foo it still gets interpreted as environment variable and the \ escape char seems to be ignored which can get annoying if you invoke something like npx nx generate ... --name=\$foo and it doesn't work.

@lydell
Copy link
Author

lydell commented May 25, 2022

@juristr Good point! That’s another case where using JSON.stringify for escaping args isn’t sufficient. npx receives --somearg=$foo and escapes that to "--somearg=$foo", but $name is still expanded inside double quotes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants