V1

github · Sep 20, 2023 · 9657491 · 9657491
1 parent dc32227
commit 9657491
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 50 deletions.
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll b/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll
@@ -88,12 +88,6 @@ module Execa {
       isExecaShellEnable(this.getParameter(1))
     }
 
-    predicate isArgumentInjectable(DataFlow::Node arg) {
-      // execa(file: string,arguments?: readonly string[],options?: Options<BufferEncodingOption>)
-      arg = this.getParameter(1).getUnknownMember().asSink() and
-      argumentIsInjectable(this.getParameter(0).asSink().getStringValue())
-    }
-
     override predicate isSync() { name = "execaSync" }
 
     override DataFlow::Node getOptionsArg() {
@@ -134,7 +128,7 @@ module Execa {
     ExecaScriptEec() { name = ["Sync", "ASync"] }
 
     override DataFlow::Node getACommandArgument() {
-      result.asExpr() = templateLiteralChildAsSink(this.asExpr()).getAChildExpr+()
+      result.asExpr() = templateLiteralChildAsSink(this.asExpr()).getChildExpr(0)
     }
 
     override predicate isShellInterpreted(DataFlow::Node arg) {
@@ -179,13 +173,32 @@ module Execa {
     string getName() { result = name }
   }
 
+  /**
+   * The system command execution nodes for `execa.execaCommand` or `execa.execaCommandSync` functions
+   */
+  class ExecaCommandExec2 extends SystemCommandExecution, DataFlow::CallNode {
+    ExecaCommandExec2() { this = API::moduleImport("execa").getMember("execaCommand").getACall() }
+
+    override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
+
+    override DataFlow::Node getArgumentList() { result = this.getArgument(0) }
+
+    override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getArgument(0) }
+
+    override predicate isSync() { none() }
+
+    override DataFlow::Node getOptionsArg() { result = this }
+  }
+
   /**
    * The system command execution nodes for `execa.execaCommand` or `execa.execaCommandSync` functions
    */
   class ExecaCommandExec extends SystemCommandExecution, ExecaCommandCall {
     ExecaCommandExec() { name = ["execaCommand", "execaCommandSync"] }
 
-    override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
+    override DataFlow::Node getACommandArgument() {
+      result = this.(DataFlow::CallNode).getArgument(0)
+    }
 
     override DataFlow::Node getArgumentList() {
       // execaCommand("echo " + sink);
@@ -212,50 +225,21 @@ module Execa {
     }
   }
 
-  // parent = left`result`
+  // Holds if left parameter is the the left child of a template literal and returns the template literal
   private TemplateLiteral templateLiteralChildAsSink(Expr left) {
     exists(TaggedTemplateExpr parent |
       parent.getTemplate() = result and
       left = parent.getChildExpr(0)
     )
   }
 
-  private class CommandsVulnerableToArgumentInjection extends string {
-    CommandsVulnerableToArgumentInjection() {
-      // Thanks to https://sonarsource.github.io/argument-injection-vectors/#+command
-      this =
-        [
-          "chrome", "git-blame", "git-clone", "git-fetch", "git-grep", "git-ls-remote", "hg",
-          "psql", "qt5", "ssh", "tar", "zip", "aria2c", "tcpdump", "sysctl", "split", "sed",
-          "pidstat", "php", "nohup", "crontab", "crontab", "crontab", "crontab", "sh", "zsh",
-          "bash", "cmd.exe", "cmd"
-        ]
-    }
-  }
-
-  // Check whether a command is vulnerable to argument injection or not
-  bindingset[cmd]
-  private predicate argumentIsInjectable(string cmd) {
-    // "cmd args" or "cmd"
-    exists(CommandsVulnerableToArgumentInjection c |
-      // full/relative path to command like `/usr/bin/cmd` or `someDir/cmd`
-      cmd.matches("%/" + c)
-      or
-      // command like `cmd args`
-      cmd.matches(c + " %")
-      or
-      // only the command `cmd`
-      cmd = c
-    )
-  }
-
-  // Check whether Execa has shell enabled options or not, get Parameter responsible for options
+  // Holds whether Execa has shell enabled options or not, get Parameter responsible for options
   private predicate isExecaShellEnable(API::Node n) {
     n.getMember("shell").asSink().asExpr().(BooleanLiteral).getValue() = "true" and
     exists(n.getMember("shell"))
   }
 
-  // Check whether Execa has shell enabled options or not, get Parameter responsible for options
+  // Holds whether Execa has shell enabled options or not, get Parameter responsible for options
   private predicate isExecaShellEnableWithExpr(Expr n) {
     exists(ObjectExpr o, Property p | o = n.getAChildExpr*() |
       o.getAChild() = p and

diff --git a/javascript/ql/test/library-tests/frameworks/Execa/Execa.expected b/javascript/ql/test/library-tests/frameworks/Execa/Execa.expected
@@ -17,9 +17,9 @@ test_FileSystemAccess
 | tst.js:37:18:37:20 | arg |
 | tst.js:39:18:39:39 | "echo 1 ... echo 2" |
 | tst.js:39:42:39:56 | { shell: true } |
-| tst.js:45:9:45:29 | { input ... sink' } |
+| tst.js:45:9:45:27 | { inputFile: file } |
 | tst.js:46:13:46:17 | 'cat' |
-| tst.js:46:20:46:40 | { input ... sink' } |
+| tst.js:46:20:46:38 | { inputFile: file } |
 | tst.js:47:13:47:18 | 'echo' |
 | tst.js:47:21:47:32 | ['example2'] |
 | tst.js:48:13:48:18 | 'echo' |
@@ -28,6 +28,10 @@ test_FileSystemAccess
 | tst.js:49:21:49:32 | ['example4'] |
 | tst.js:49:35:49:47 | { all: true } |
 test_MissingFileSystemAccess
+| tst.js:43:35:43:38 | file |
+| tst.js:47:46:47:49 | file |
+| tst.js:48:46:48:49 | file |
+| tst.js:49:58:49:61 | file |
 test_SystemCommandExecution
 | tst.js:1:71:1:71 | $ |
 | tst.js:4:7:4:7 | $ |
@@ -56,8 +60,8 @@ test_SystemCommandExecution
 | tst.js:39:1:39:57 | execaCo ... true }) |
 | tst.js:43:7:43:7 | $ |
 | tst.js:45:7:45:7 | $ |
-| tst.js:45:7:45:30 | $({ inp ... ink' }) |
-| tst.js:46:7:46:41 | execa(' ... ink' }) |
+| tst.js:45:7:45:28 | $({ inp ... file }) |
+| tst.js:46:7:46:39 | execa(' ... file }) |
 | tst.js:47:7:47:33 | execa(' ... ple2']) |
 | tst.js:48:7:48:33 | execa(' ... ple3']) |
 | tst.js:49:7:49:48 | execa(' ... true }) |

diff --git a/javascript/ql/test/library-tests/frameworks/Execa/tst.js b/javascript/ql/test/library-tests/frameworks/Execa/tst.js
@@ -40,10 +40,10 @@ execaCommandSync("echo 1 " + "; echo 2", { shell: true });
 
 // FileSystemAccess
 // Piping stdout to a file
-await $`echo example8`.pipeStdout('tmp')
+await $`echo example8`.pipeStdout(file)
 // Piping stdin from a file
-await $({ inputFile: 'sink' })`cat`
-await execa('cat', { inputFile: 'sink' });
-await execa('echo', ['example2']).pipeStdout('tmpdir/sink.txt');
-await execa('echo', ['example3']).pipeStderr('tmpdir/sink.txt');
-await execa('echo', ['example4'], { all: true }).pipeAll('tmpdir/sink.txt');
+await $({ inputFile: file })`cat`
+await execa('cat', { inputFile: file });
+await execa('echo', ['example2']).pipeStdout(file);
+await execa('echo', ['example3']).pipeStderr(file);
+await execa('echo', ['example4'], { all: true }).pipeAll(file);