Command Injection Vulnerability in win-fork/win-spawn Packages
State Informative (Closed)
Disclosed publicly 2018-08-10T13:08:53.777Z
Reported To
Weakness Command Injection - Generic
Bounty
Collapse


Timeline
submitted a report to Node.js third-party modules .
2018-08-06T11:41:27.206Z

I would like to report a command injection vulnerability in win-fork and win-spawn packages.
It allows an attacker to inject multiple commands in exec-like manner.

Module

module name: win-spawn
version: 2.0.0
npm page: https://www.npmjs.com/package/win-spawn
npm page: https://www.npmjs.com/package/win-fork

Module Description

Spawn for node.js but in a way that works regardless of which OS you're using. Use this if you want to use spawn with a JavaScript file. It works by explicitly invoking node on windows. It also shims support for environment variable setting by attempting to parse the command with a regex. Since all modification is wrapped in if (os === 'Windows_NT') it can be safely used on non-windows systems and will not break anything.

Module Stats

21,929+36,468 downloads in the last week

Vulnerability

Vulnerability Description

Even though this module is advertised to work like spawn, on windows, it works like exec.

Steps To Reproduce:

To check the params passed to cmd.exe:

var os = require('os').type = function() {return "Windows_NT"};
require("child_process").spawn = function(a, b) { console.log(a); console.log(b)};
var spawn = require("win-fork");
spawn('dir C:// && date /T', [], {stdio: 'inherit'});

It effectively runs "cmd /c 'dir C:// && date /T'" which allow the attacker to run both the commands. Moreover, I believe parameters to win-spawn/win-fork may also be used for injection, but I did not investigate this further.

Patch

N/A at a minimum, document this behaviour in the package's documentation.

Wrap up

  • I contacted the maintainer to let them know: N
  • I opened an issue in the related repository: N

Impact

This issue is more a documentation/API issue. The package should state clearly what it does and alert its dependents that on windows, the parameters should be treated as parameters to exec.

Regards,
Frans

  • 0 attachments:
vdeturckheim_dev Activities::Comment
2018-08-06T18:00:06.526Z
Hello, Thanks for reporting this to us. Someone will quickly look at this report and triage it.


lirantal Activities::BugTriaged
2018-08-06T18:30:40.079Z


lirantal Activities::Comment
2018-08-06T18:36:55.242Z
Hi @cris_semmle, Thanks for reporting this issue. I was able to reproduce and confirm the issue as you described and will triage this report as vulnerability. I will invite the package maintainer to this issue. A couple of things to note: 1. Looks like win-fork redirects to win-spawn actually, and win-spawn has a clear deprecation message on both the npmjs page and on the project's README. 2. I agree with your impact clause that the issue is probably more of a documentation but very relevant for people who pay attention.


lirantal Activities::Comment
2018-08-10T11:17:43.341Z
@cris_semmle given the above deprecation notices, do you think we can just resolve this as an informative report and you can individually report this information in the public by opening an issue in the repository ?


cris_semmle Activities::Comment
2018-08-10T11:32:11.156Z
Sure, also considering that the package is continuously decreasing in popularity.


lirantal Activities::Comment
2018-08-10T11:56:23.273Z
Ok thanks, I'll go ahead and update now. Would you like me to also disclose this report publicly?


cris_semmle Activities::Comment
2018-08-10T12:43:22.096Z
yes, that is fine with me


lirantal Activities::BugInformative
2018-08-10T12:44:02.650Z
Closing report.


lirantal Activities::AgreedOnGoingPublic
2018-08-10T12:44:10.924Z


lirantal Activities::ChangedScope
2018-08-10T13:08:41.189Z


lirantal Activities::ManuallyDisclosed
2018-08-10T13:08:53.661Z