Merge pull request #14294 from am0o0/amammad-js-CodeInjection_execa

JS: provide command execution sinks for execa package

javascript/ql/src/experimental/semmle/javascript/Execa.qll

@@ -0,0 +1,211 @@
+/**
+ * Models the `execa` library in terms of `FileSystemAccess` and `SystemCommandExecution`.
+ */
+
+import javascript
+
+/**
+ * Provide model for [Execa](https://github.com/sindresorhus/execa) package
+ */
+module Execa {
+  /**
+   * The Execa input file read and output file write
+   */
+  class ExecaFileSystemAccess extends FileSystemReadAccess, DataFlow::Node {
+    API::Node execaArg;
+    boolean isPipedToFile;
+
+    ExecaFileSystemAccess() {
+      (
+        execaArg = API::moduleImport("execa").getMember("$").getParameter(0) and
+        isPipedToFile = false
+        or
+        execaArg =
+          API::moduleImport("execa")
+              .getMember(["execa", "execaCommand", "execaCommandSync", "execaSync"])
+              .getParameter([0, 1, 2]) and
+        isPipedToFile = false
+        or
+        execaArg =
+          API::moduleImport("execa")
+              .getMember(["execa", "execaCommand", "execaCommandSync", "execaSync"])
+              .getReturn()
+              .getMember(["pipeStdout", "pipeAll", "pipeStderr"])
+              .getParameter(0) and
+        isPipedToFile = true
+      ) and
+      this = execaArg.asSink()
+    }
+
+    override DataFlow::Node getADataNode() { none() }
+
+    override DataFlow::Node getAPathArgument() {
+      result = execaArg.getMember("inputFile").asSink() and isPipedToFile = false
+      or
+      result = execaArg.asSink() and isPipedToFile = true
+    }
+  }
+
+  /**
+   * A call to `execa.execa` or `execa.execaSync`
+   */
+  class ExecaCall extends API::CallNode {
+    boolean isSync;
+
+    ExecaCall() {
+      this = API::moduleImport("execa").getMember("execa").getACall() and
+      isSync = false
+      or
+      this = API::moduleImport("execa").getMember("execaSync").getACall() and
+      isSync = true
+    }
+  }
+
+  /**
+   * The system command execution nodes for `execa.execa` or `execa.execaSync` functions
+   */
+  class ExecaExec extends SystemCommandExecution, ExecaCall {
+    ExecaExec() { isSync = [false, true] }
+
+    override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
+
+    override predicate isShellInterpreted(DataFlow::Node arg) {
+      // if shell: true then first and second args are sinks
+      // options can be third argument
+      arg = [this.getArgument(0), this.getParameter(1).getUnknownMember().asSink()] and
+      isExecaShellEnable(this.getParameter(2))
+      or
+      // options can be second argument
+      arg = this.getArgument(0) and
+      isExecaShellEnable(this.getParameter(1))
+    }
+
+    override DataFlow::Node getArgumentList() {
+      // execa(cmd, [arg]);
+      exists(DataFlow::Node arg | arg = this.getArgument(1) |
+        // if it is a object then it is a option argument not command argument
+        result = arg and not arg.asExpr() instanceof ObjectExpr
+      )
+    }
+
+    override predicate isSync() { isSync = true }
+
+    override DataFlow::Node getOptionsArg() {
+      result = this.getLastArgument() and result.asExpr() instanceof ObjectExpr
+    }
+  }
+
+  /**
+   * A call to `execa.$` or `execa.$.sync` or `execa.$({})` or `execa.$.sync({})` tag functions
+   */
+  private class ExecaScriptCall extends API::CallNode {
+    boolean isSync;
+
+    ExecaScriptCall() {
+      exists(API::Node script |
+        script =
+          [
+            API::moduleImport("execa").getMember("$"),
+            API::moduleImport("execa").getMember("$").getReturn()
+          ]
+      |
+        this = script.getACall() and
+        isSync = false
+        or
+        this = script.getMember("sync").getACall() and
+        isSync = true
+      )
+    }
+  }
+
+  /**
+   * The system command execution nodes for `execa.$` or `execa.$.sync` tag functions
+   */
+  class ExecaScript extends SystemCommandExecution, ExecaScriptCall {
+    ExecaScript() { isSync = [false, true] }
+
+    override DataFlow::Node getACommandArgument() {
+      result = this.getParameter(1).asSink() and
+      not isTaggedTemplateFirstChildAnElement(this.getParameter(1).asSink().asExpr().getParent())
+    }
+
+    override predicate isShellInterpreted(DataFlow::Node arg) {
+      isExecaShellEnable(this.getParameter(0)) and
+      arg = this.getAParameter().asSink()
+    }
+
+    override DataFlow::Node getArgumentList() {
+      result = this.getParameter(any(int i | i >= 1)).asSink() and
+      isTaggedTemplateFirstChildAnElement(this.getParameter(1).asSink().asExpr().getParent())
+      or
+      result = this.getParameter(any(int i | i >= 2)).asSink() and
+      not isTaggedTemplateFirstChildAnElement(this.getParameter(1).asSink().asExpr().getParent())
+    }
+
+    override DataFlow::Node getOptionsArg() { result = this.getParameter(0).asSink() }
+
+    override predicate isSync() { isSync = true }
+  }
+
+  /**
+   * A call to `execa.execaCommandSync` or `execa.execaCommand`
+   */
+  private class ExecaCommandCall extends API::CallNode {
+    boolean isSync;
+
+    ExecaCommandCall() {
+      this = API::moduleImport("execa").getMember("execaCommandSync").getACall() and
+      isSync = true
+      or
+      this = API::moduleImport("execa").getMember("execaCommand").getACall() and
+      isSync = false
+    }
+  }
+
+  /**
+   * The system command execution nodes for `execa.execaCommand` or `execa.execaCommandSync` functions
+   */
+  class ExecaCommandExec extends SystemCommandExecution, ExecaCommandCall {
+    ExecaCommandExec() { isSync = [false, true] }
+
+    override DataFlow::Node getACommandArgument() {
+      result = this.(DataFlow::CallNode).getArgument(0)
+    }
+
+    override DataFlow::Node getArgumentList() {
+      // execaCommand(`${cmd} ${arg}`);
+      result.asExpr() = this.getParameter(0).asSink().asExpr().getAChildExpr() and
+      not result.asExpr() = this.getArgument(0).asExpr().getChildExpr(0)
+    }
+
+    override predicate isShellInterpreted(DataFlow::Node arg) {
+      // execaCommandSync(`${cmd} ${arg}`, {shell: true})
+      arg.asExpr() = this.getArgument(0).asExpr().getAChildExpr+() and
+      isExecaShellEnable(this.getParameter(1))
+      or
+      // there is only one argument that is constructed in previous nodes,
+      // it makes sanitizing really hard to select whether it is vulnerable to argument injection or not
+      arg = this.getParameter(0).asSink() and
+      not exists(this.getArgument(0).asExpr().getChildExpr(1))
+    }
+
+    override predicate isSync() { isSync = true }
+
+    override DataFlow::Node getOptionsArg() {
+      result = this.getLastArgument() and result.asExpr() instanceof ObjectExpr
+    }
+  }
+
+  /** Gets a TemplateLiteral and check if first child is a template element */
+  private predicate isTaggedTemplateFirstChildAnElement(TemplateLiteral templateLit) {
+    exists(templateLit.getChildExpr(0).(TemplateElement))
+  }
+
+  /**
+   * Holds whether Execa has shell enabled options or not, get Parameter responsible for options
+   */
+  pragma[inline]
+  private predicate isExecaShellEnable(API::Node n) {
+    n.getMember("shell").asSink().asExpr().(BooleanLiteral).getValue() = "true"
+  }
+}

javascript/ql/test/experimental/Execa/CommandInjection/tests.expected

@@ -0,0 +1,22 @@
+passingPositiveTests
+| PASSED | CommandInjection | tests.js:11:46:11:70 | // test ... jection |
+| PASSED | CommandInjection | tests.js:12:43:12:67 | // test ... jection |
+| PASSED | CommandInjection | tests.js:13:63:13:87 | // test ... jection |
+| PASSED | CommandInjection | tests.js:14:62:14:86 | // test ... jection |
+| PASSED | CommandInjection | tests.js:15:60:15:84 | // test ... jection |
+| PASSED | CommandInjection | tests.js:17:45:17:69 | // test ... jection |
+| PASSED | CommandInjection | tests.js:18:42:18:66 | // test ... jection |
+| PASSED | CommandInjection | tests.js:19:62:19:86 | // test ... jection |
+| PASSED | CommandInjection | tests.js:20:63:20:87 | // test ... jection |
+| PASSED | CommandInjection | tests.js:21:60:21:84 | // test ... jection |
+| PASSED | CommandInjection | tests.js:23:43:23:67 | // test ... jection |
+| PASSED | CommandInjection | tests.js:24:40:24:64 | // test ... jection |
+| PASSED | CommandInjection | tests.js:25:40:25:64 | // test ... jection |
+| PASSED | CommandInjection | tests.js:26:60:26:84 | // test ... jection |
+| PASSED | CommandInjection | tests.js:28:41:28:65 | // test ... jection |
+| PASSED | CommandInjection | tests.js:29:58:29:82 | // test ... jection |
+| PASSED | CommandInjection | tests.js:31:51:31:75 | // test ... jection |
+| PASSED | CommandInjection | tests.js:32:68:32:92 | // test ... jection |
+| PASSED | CommandInjection | tests.js:34:49:34:73 | // test ... jection |
+| PASSED | CommandInjection | tests.js:35:66:35:90 | // test ... jection |
+failingPositiveTests

javascript/ql/test/experimental/Execa/CommandInjection/tests.js

@@ -0,0 +1,36 @@
+import { execa, execaSync, execaCommand, execaCommandSync, $ } from 'execa';
+import http from 'node:http'
+import url from 'url'
+
+http.createServer(async function (req, res) {
+    let cmd = url.parse(req.url, true).query["cmd"][0];
+    let arg1 = url.parse(req.url, true).query["arg1"];
+    let arg2 = url.parse(req.url, true).query["arg2"];
+    let arg3 = url.parse(req.url, true).query["arg3"];
+
+    await $`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+    await $`ssh ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+    $({ shell: false }).sync`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+    $({ shell: true }).sync`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+    $({ shell: false }).sync`ssh ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+
+    $.sync`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+    $.sync`ssh ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
+    await $({ shell: true })`${cmd} ${arg1} ${arg2} ${arg3}` // test: CommandInjection
+    await $({ shell: false })`${cmd} ${arg1} ${arg2} ${arg3}` // test: CommandInjection
+    await $({ shell: false })`ssh ${arg1} ${arg2} ${arg3}` // test: CommandInjection
+
+    await execa(cmd, [arg1, arg2, arg3]); // test: CommandInjection
+    await execa(cmd, { shell: true }); // test: CommandInjection
+    await execa(cmd, { shell: true }); // test: CommandInjection
+    await execa(cmd, [arg1, arg2, arg3], { shell: true }); // test: CommandInjection
+
+    execaSync(cmd, [arg1, arg2, arg3]); // test: CommandInjection
+    execaSync(cmd, [arg1, arg2, arg3], { shell: true }); // test: CommandInjection
+
+    await execaCommand(cmd + arg1 + arg2 + arg3); // test: CommandInjection
+    await execaCommand(cmd + arg1 + arg2 + arg3, { shell: true }); // test: CommandInjection
+
+    execaCommandSync(cmd + arg1 + arg2 + arg3); // test: CommandInjection
+    execaCommandSync(cmd + arg1 + arg2 + arg3, { shell: true }); // test: CommandInjection
+});

javascript/ql/test/experimental/Execa/CommandInjection/tests.ql

@@ -0,0 +1,38 @@
+import javascript
+
+class InlineTest extends LineComment {
+  string tests;
+
+  InlineTest() { tests = this.getText().regexpCapture("\\s*test:(.*)", 1) }
+
+  string getPositiveTest() {
+    result = tests.trim().splitAt(",").trim() and not result.matches("!%")
+  }
+
+  predicate hasPositiveTest(string test) { test = this.getPositiveTest() }
+
+  predicate inNode(DataFlow::Node n) {
+    this.getLocation().getFile() = n.getFile() and
+    this.getLocation().getStartLine() = n.getStartLine()
+  }
+}
+
+import experimental.semmle.javascript.Execa
+
+query predicate passingPositiveTests(string res, string expectation, InlineTest t) {
+  res = "PASSED" and
+  t.hasPositiveTest(expectation) and
+  expectation = "CommandInjection" and
+  exists(SystemCommandExecution n |
+    t.inNode(n.getArgumentList()) or t.inNode(n.getACommandArgument())
+  )
+}
+
+query predicate failingPositiveTests(string res, string expectation, InlineTest t) {
+  res = "FAILED" and
+  t.hasPositiveTest(expectation) and
+  expectation = "CommandInjection" and
+  not exists(SystemCommandExecution n |
+    t.inNode(n.getArgumentList()) or t.inNode(n.getACommandArgument())
+  )
+}

javascript/ql/test/experimental/Execa/PathInjection/tests.expected

@@ -0,0 +1,6 @@
+passingPositiveTests
+| PASSED | PathInjection | tests.js:9:43:9:64 | // test ... jection |
+| PASSED | PathInjection | tests.js:12:50:12:71 | // test ... jection |
+| PASSED | PathInjection | tests.js:15:61:15:82 | // test ... jection |
+| PASSED | PathInjection | tests.js:18:73:18:94 | // test ... jection |
+failingPositiveTests

javascript/ql/test/experimental/Execa/PathInjection/tests.js

@@ -0,0 +1,19 @@
+import { execa, $ } from 'execa';
+import http from 'node:http'
+import url from 'url'
+
+http.createServer(async function (req, res) {
+    let filePath = url.parse(req.url, true).query["filePath"][0];
+
+    // Piping to stdin from a file
+    await $({ inputFile: filePath })`cat` // test: PathInjection
+
+    // Piping to stdin from a file
+    await execa('cat', { inputFile: filePath }); // test: PathInjection
+
+    // Piping Stdout to file
+    await execa('echo', ['example3']).pipeStdout(filePath); // test: PathInjection
+
+    // Piping all of command output to file
+    await execa('echo', ['example4'], { all: true }).pipeAll(filePath); // test: PathInjection
+});

javascript/ql/test/experimental/Execa/PathInjection/tests.ql

@@ -0,0 +1,34 @@
+import javascript
+
+class InlineTest extends LineComment {
+  string tests;
+
+  InlineTest() { tests = this.getText().regexpCapture("\\s*test:(.*)", 1) }
+
+  string getPositiveTest() {
+    result = tests.trim().splitAt(",").trim() and not result.matches("!%")
+  }
+
+  predicate hasPositiveTest(string test) { test = this.getPositiveTest() }
+
+  predicate inNode(DataFlow::Node n) {
+    this.getLocation().getFile() = n.getFile() and
+    this.getLocation().getStartLine() = n.getStartLine()
+  }
+}
+
+import experimental.semmle.javascript.Execa
+
+query predicate passingPositiveTests(string res, string expectation, InlineTest t) {
+  res = "PASSED" and
+  t.hasPositiveTest(expectation) and
+  expectation = "PathInjection" and
+  exists(FileSystemReadAccess n | t.inNode(n.getAPathArgument()))
+}
+
+query predicate failingPositiveTests(string res, string expectation, InlineTest t) {
+  res = "FAILED" and
+  t.hasPositiveTest(expectation) and
+  expectation = "PathInjection" and
+  not exists(FileSystemReadAccess n | t.inNode(n.getAPathArgument()))
+}