Security issue

[lirantal]

Description

The pullit npm package makes insecure use of shell execution API (i.e: exec() or execSync()) which is vulnerable to a malicious user input based on a remote branch name on the GitHub platform, that can be set by a 3rd party, hence luring an innocent user to use the pullit module on the target branch and result in remote command execution exploit.

Steps To Reproduce:

The pullit project has a set of exec() calls to git commands which may end up in originating from user input in terms of a carefully created remote branch name on GitHub, which pullit pulls branch names from.

Re-construct of a flow that results in a remote command execution on the user running pullit: 

1. Create a branch that could potentially terminate an exec() command and concatenate to it a new command:
   1. git checkout -b ";{echo,hello,world}>/tmp/c”
2. Push it to GitHub and create a pull request with this branch name
3. Run pullit from command line, select the relevant pull request to checkout locally
4. Read the contents of /tmp/c

Patch

See below for patch to fix the problem:

pullit-security-rce.patch:

diff --git a/src/index.js b/src/index.js
index 3a34831..9bffd0d 100644
--- a/src/index.js
+++ b/src/index.js
@@ -1,7 +1,7 @@
 const GitHubApi = require('github');
 const Menu = require('terminal-menu');
 const {
-  execSync
+  execFileSync
 } = require('child_process');
 const parse = require('parse-github-repo-url');

@@ -12,7 +12,7 @@ class Pullit {
   }

   init() {
-    const url = execSync(`git config --get remote.origin.url`, {
+    const url = execFileSync('git', ['config', '--get', 'remote.origin.url'], {
       encoding: 'utf8'
     }).trim();

@@ -34,8 +34,11 @@ class Pullit {
       })
       .then(res => {
         const branch = res.data.head.ref;
-        execSync(
-          `git fetch origin pull/${id}/head:${branch} && git checkout ${branch}`
+        execFileSync(
+          'git', ['fetch', 'origin', `pull/${id}/head:${branch}`]
+        );
+        execFileSync(
+          'git', ['checkout', branch]
         );
       })
       .catch(err => {

[jkup]

@lirantal wow thanks for reporting this! I'll merge that patch in ASAP.

[lirantal]

Hey, no problems.

I guess you weren't aware but just to state that me and Karen from Snyk.io tried to reach out to you since October 2017 about this vulnerability. Probably should check your email filters or something :-)

[jkup]

I get a lot of spam about "problems found with this or that github repo" so I do filter those out. It really stinks because you all have a very legitimate one. IMO it's a way better approach to open an issue or a PR on github directly.

Thanks again!

[karenyavine]

Hey there!

Glad to see this getting attention! Wanted to update that the vulnerability was published to our database here: https://snyk.io/vuln/npm:pullit:20180214
I'll update the vulnerability info once there is a fix for this.

Also, from my experience, most people prefer being notified via email through a responsible disclosure, giving them enough time to fix the issue before it goes public.

Cheers,
Karen

[lirantal]

@karenyavine btw, don't you think a more proper CWE for the logged snyk issue is OS Command Injection?

[jkup]

Thanks again all, merged patch and pushed to npm.

[lirantal]

Thanks Jon.
Can you please jump on the Hacker1 conversation to follow-up with us about the vulnerability?