JS: generalize js/command-line-injection to handle ConstantString

Author: Esben Sparre Andreasen

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

@@ -77,7 +77,7 @@ module CommandInjection {
     override predicate isSource(DataFlow::Node nd) {
       nd instanceof DataFlow::ArrayCreationNode
       or
-      exists (StringLiteral shell | shellCmd(shell, _) |
+      exists (ConstantString shell | shellCmd(shell, _) |
         nd = DataFlow::valueNode(shell)
       )
     }
@@ -96,14 +96,14 @@ module CommandInjection {
    * That is, either `shell` is a Unix shell (`sh` or similar) and
    * `arg` is `"-c"`, or `shell` is `cmd.exe` and `arg` is `"/c"`.
    */
-  private predicate shellCmd(StringLiteral shell, string arg) {
-    exists (string s | s = shell.getValue() |
+  private predicate shellCmd(ConstantString shell, string arg) {
+    exists (string s | s = shell.getStringValue() |
       (s = "sh" or s = "bash" or s = "/bin/sh" or s = "/bin/bash")
       and
       arg = "-c"
     )
     or
-    exists (string s | s = shell.getValue().toLowerCase() |
+    exists (string s | s = shell.getStringValue().toLowerCase() |
       (s = "cmd" or s = "cmd.exe")
       and
       (arg = "/c" or arg = "/C")
@@ -126,7 +126,7 @@ module CommandInjection {
    */
   private predicate indirectCommandInjection(DataFlow::Node sink, SystemCommandExecution sys) {
     exists (ArgumentListTracking cfg, DataFlow::ArrayCreationNode args,
-            StringLiteral shell, string dashC |
+            ConstantString shell, string dashC |
       shellCmd(shell, dashC) and
       cfg.hasFlow(DataFlow::valueNode(shell), sys.getACommandArgument()) and
       cfg.hasFlow(args, sys.getArgumentList()) and

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

@@ -29,10 +29,15 @@ nodes
 | child_process-test.js:44:30:44:33 | args |
 | child_process-test.js:46:9:46:12 | "sh" |
 | child_process-test.js:46:15:46:18 | args |
-| child_process-test.js:49:14:49:16 | cmd |
-| child_process-test.js:49:19:49:22 | args |
-| child_process-test.js:50:12:50:14 | cmd |
-| child_process-test.js:50:17:50:20 | args |
+| child_process-test.js:48:9:48:17 | args |
+| child_process-test.js:48:16:48:17 | [] |
+| child_process-test.js:50:15:50:17 | cmd |
+| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
+| child_process-test.js:51:35:51:38 | args |
+| child_process-test.js:55:14:55:16 | cmd |
+| child_process-test.js:55:19:55:22 | args |
+| child_process-test.js:56:12:56:14 | cmd |
+| child_process-test.js:56:17:56:20 | args |
 edges
 | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd |
 | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:18:17:18:19 | cmd |
@@ -44,6 +49,7 @@ edges
 | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:25:21:25:23 | cmd |
 | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:39:26:39:28 | cmd |
 | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:43:15:43:17 | cmd |
+| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:50:15:50:17 | cmd |
 | child_process-test.js:6:15:6:38 | url.par ... , true) | child_process-test.js:6:15:6:44 | url.par ... ).query |
 | child_process-test.js:6:15:6:44 | url.par ... ).query | child_process-test.js:6:15:6:49 | url.par ... ry.path |
 | child_process-test.js:6:15:6:49 | url.par ... ry.path | child_process-test.js:6:9:6:49 | cmd |
@@ -58,10 +64,12 @@ edges
 | child_process-test.js:41:9:41:17 | args | child_process-test.js:44:30:44:33 | args |
 | child_process-test.js:41:9:41:17 | args | child_process-test.js:46:15:46:18 | args |
 | child_process-test.js:41:16:41:17 | [] | child_process-test.js:41:9:41:17 | args |
-| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:49:14:49:16 | cmd |
-| child_process-test.js:46:15:46:18 | args | child_process-test.js:49:19:49:22 | args |
-| child_process-test.js:49:14:49:16 | cmd | child_process-test.js:50:12:50:14 | cmd |
-| child_process-test.js:49:19:49:22 | args | child_process-test.js:50:17:50:20 | args |
+| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:55:14:55:16 | cmd |
+| child_process-test.js:46:15:46:18 | args | child_process-test.js:55:19:55:22 | args |
+| child_process-test.js:48:9:48:17 | args | child_process-test.js:51:35:51:38 | args |
+| child_process-test.js:48:16:48:17 | [] | child_process-test.js:48:9:48:17 | args |
+| child_process-test.js:55:14:55:16 | cmd | child_process-test.js:56:12:56:14 | cmd |
+| child_process-test.js:55:19:55:22 | args | child_process-test.js:56:17:56:20 | args |
 #select
 | child_process-test.js:17:13:17:15 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:17:13:17:15 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
 | child_process-test.js:18:17:18:19 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:18:17:18:19 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
@@ -73,4 +81,5 @@ edges
 | child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
 | child_process-test.js:39:5:39:31 | cp.spaw ...  cmd ]) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:39:26:39:28 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
 | child_process-test.js:44:5:44:34 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
-| child_process-test.js:50:3:50:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
+| child_process-test.js:51:5:51:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:50:15:50:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
+| child_process-test.js:56:3:56:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |

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

@@ -44,6 +44,12 @@ var server = http.createServer(function(req, res) {
     cp.execFile("/bin/bash", args); // NOT OK
 
     run("sh", args);
+
+    let args = [];
+    args[0] = `-` + "c";
+    args[1] = cmd;
+    cp.execFile(`/bin` + "/bash", args); // NOT OK
+
 });
 
 function run(cmd, args) {