git-diff
git-diff copied to clipboard
danday74
Shell injection vulnerability
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.
To add to this: using execFile or spawn instead of exec will prevent most command injection vulnerabities; instead of passing in a concatenated string, you pass in a list of arguments.
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)
+1 on this.
I used git-diff in 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 exec call 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!
erased the contents of my project's node_modules directory
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.
Oh hey, nice catch. That's probably what happened!
EDIT: I see npm ci and npm install in 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.