Repository navigation
Shell injection vulnerability #6
Description
JSON.stringify does not protect against shell command injection. Please use proper child_process APIs and send the data via an stdin pipe.
Thanks to @sehrope for pointing out this bug on IRC.
Activity
joepie91 commentedon Aug 12, 2019
To add to this: using
execFileorspawninstead ofexecwill prevent most command injection vulnerabities; instead of passing in a concatenated string, you pass in a list of arguments.danday74 commentedon Aug 19, 2019
Anyone wanna raise a PR for this? I am busy ATM - If anyone has time, unit tests must continue to pass with 100% coverage (unless a valid reason is given why coverage is not 100% and the relevant block should be commented out with istanbul ignore - happy to add contributors as npm contributors also so they can publish)
AverageHelper commentedon Sep 10, 2022
+1 on this.
I used
git-diffin a project of mine for pretty output in a release script. Worked reliably.When I copied that script to another project, something about my changelog file and this
execcall erased the contents of my project's node_modules directory, everything but its .bin subfolder. I'm not sure what else might've borked, and even this issue wouldn't reliably occur (I'm no computer forensics researcher lol), I only know that that injection can be nasty!mk-pmb commentedon Sep 11, 2022
It's probably because the change log contains concatenated shell commands like
&& npm run build:clean &&. I wouldn't be surprised if git-diff runs any command quoted in backticks.AverageHelper commentedon Sep 11, 2022
Oh hey, nice catch. That's probably what happened!
EDIT: I see
npm ciandnpm installin there as well. All of those together take a while, which explains why the spicy "diff" took so long. I remember terminating the script early a few times before I discovered node_modules was messed, so I guess I must've caught the script in the middle of reinstalling everything.