Merge pull request github#5606 from erik-krogh/shellInput

Approved by esbena

Author: codeql-ci

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -190,6 +190,27 @@ module UnsafeShellCommandConstruction {
     override DataFlow::Node getAlertLocation() { result = this }
   }
 
+  /**
+   * A joined path (`path.{resolve/join}(..)`) that is later executed as a shell command.
+   * Joining a path is similar to string concatenation that automatically inserts slashes.
+   */
+  class JoinedPathEndingInCommandExecutionSink extends Sink {
+    DataFlow::MethodCallNode joinCall;
+    SystemCommandExecution sys;
+
+    JoinedPathEndingInCommandExecutionSink() {
+      this = joinCall.getAnArgument() and
+      joinCall = DataFlow::moduleMember("path", ["resolve", "join"]).getACall() and
+      joinCall = isExecutedAsShellCommand(DataFlow::TypeBackTracker::end(), sys)
+    }
+
+    override string getSinkType() { result = "Path concatenation" }
+
+    override SystemCommandExecution getCommandExecution() { result = sys }
+
+    override DataFlow::Node getAlertLocation() { result = this }
+  }
+
   /**
    * A sanitizer like: "'"+name.replace(/'/g,"'\\''")+"'"
    * Which sanitizes on Unix.

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

@@ -237,6 +237,11 @@ nodes
 | lib/lib.js:446:20:446:23 | name |
 | lib/lib.js:447:25:447:28 | name |
 | lib/lib.js:447:25:447:28 | name |
+| lib/lib.js:477:33:477:38 | config |
+| lib/lib.js:477:33:477:38 | config |
+| lib/lib.js:478:27:478:32 | config |
+| lib/lib.js:478:27:478:46 | config.installedPath |
+| lib/lib.js:478:27:478:46 | config.installedPath |
 | lib/subLib/index.js:3:28:3:31 | name |
 | lib/subLib/index.js:3:28:3:31 | name |
 | lib/subLib/index.js:4:22:4:25 | name |
@@ -529,6 +534,10 @@ edges
 | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name |
 | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name |
 | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name |
+| lib/lib.js:477:33:477:38 | config | lib/lib.js:478:27:478:32 | config |
+| lib/lib.js:477:33:477:38 | config | lib/lib.js:478:27:478:32 | config |
+| lib/lib.js:478:27:478:32 | config | lib/lib.js:478:27:478:46 | config.installedPath |
+| lib/lib.js:478:27:478:32 | config | lib/lib.js:478:27:478:46 | config.installedPath |
 | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
 | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
 | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
@@ -605,5 +614,6 @@ edges
 | lib/lib.js:436:19:436:22 | last | lib/lib.js:414:40:414:43 | name | lib/lib.js:436:19:436:22 | last | $@ based on $@ is later used in $@. | lib/lib.js:436:19:436:22 | last | Shell argument | lib/lib.js:414:40:414:43 | name | library input | lib/lib.js:428:2:428:70 | spawn(" ... WN_OPT) | shell command |
 | lib/lib.js:442:12:442:27 | "rm -rf " + name | lib/lib.js:441:39:441:42 | name | lib/lib.js:442:24:442:27 | name | $@ based on $@ is later used in $@. | lib/lib.js:442:12:442:27 | "rm -rf " + name | String concatenation | lib/lib.js:441:39:441:42 | name | library input | lib/lib.js:442:2:442:28 | asyncEx ... + name) | shell command |
 | lib/lib.js:447:13:447:28 | "rm -rf " + name | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name | $@ based on $@ is later used in $@. | lib/lib.js:447:13:447:28 | "rm -rf " + name | String concatenation | lib/lib.js:446:20:446:23 | name | library input | lib/lib.js:447:3:447:29 | asyncEx ... + name) | shell command |
+| lib/lib.js:478:27:478:46 | config.installedPath | lib/lib.js:477:33:477:38 | config | lib/lib.js:478:27:478:46 | config.installedPath | $@ based on $@ is later used in $@. | lib/lib.js:478:27:478:46 | config.installedPath | Path concatenation | lib/lib.js:477:33:477:38 | config | library input | lib/lib.js:479:12:479:20 | exec(cmd) | shell command |
 | lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:3:28:3:31 | name | library input | lib/subLib/index.js:4:2:4:26 | cp.exec ... + name) | shell command |
 | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:7:32:7:35 | name | library input | lib/subLib/index.js:8:2:8:26 | cp.exec ... + name) | shell command |

javascript/ql/test/query-tests/Security/CWE-078/lib/lib.js

@@ -468,3 +468,13 @@ Object.defineProperties(
 		)
 	)
 );
+
+const path = require('path');
+const {promisify} = require('util');
+
+const exec = promisify(require('child_process').exec);
+
+module.exports = function check(config) {
+    const cmd = path.join(config.installedPath, 'myBinary -v'); // NOT OK
+    return exec(cmd);
+}