expand the explanation to include with arguments make the commands vulnerable

Author: erik-krogh

javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionCustomizations.qll

@@ -25,6 +25,13 @@ module SecondOrderCommandInjection {
       or
       this = "hg" and exists(result)
     }
+
+    /** Gets an example argument that can cause this command to execute arbitrary code. */
+    string getVulnerableArgumentExample() {
+      this = "git" and result = "--upload-pack"
+      or
+      this = "hg" and result = "--config=alias.<alias>=<command>"
+    }
   }
 
   /** A source for second order command injection. */
@@ -65,6 +72,26 @@ module SecondOrderCommandInjection {
   abstract class Sink extends DataFlow::Node {
     /** Gets a label for which this is a sink. */
     abstract DataFlow::FlowLabel getALabel();
+
+    /** Gets the command getting invoked. I.e. `git` or `hg`. */
+    abstract string getCommand();
+
+    /**
+     * Gets an example argument for the comand that allows for second order command injection.
+     * E.g. `--upload-pack` for `git`.
+     */
+    abstract string getVulnerableArgumentExample();
+  }
+
+  /**
+   * A sink that invokes a command described by the `VulnerableCommand` class.
+   */
+  abstract class VulnerableCommandSink extends Sink {
+    VulnerableCommand cmd;
+
+    override string getCommand() { result = cmd }
+
+    override string getVulnerableArgumentExample() { result = cmd.getVulnerableArgumentExample() }
   }
 
   /** A call that (indirectly) executes a shell command with a list of arguments. */
@@ -131,9 +158,9 @@ module SecondOrderCommandInjection {
   }
 
   /** An argument to an invocation of `git`/`hg` that can cause second order command injection. */
-  class ArgSink extends Sink {
+  class ArgSink extends VulnerableCommandSink {
     ArgSink() {
-      exists(DataFlow::ArrayCreationNode args, VulnerableCommand cmd |
+      exists(DataFlow::ArrayCreationNode args |
         args = usedAsVersionControlArgs(DataFlow::TypeBackTracker::end(), _, cmd)
       |
         this = [args.getAnElement(), args.getASpreadArgument()] and
@@ -149,11 +176,9 @@ module SecondOrderCommandInjection {
   /**
    * An arguments array given to an invocation of `git` or `hg` that can cause second order command injection.
    */
-  class ArgsArraySink extends Sink {
+  class ArgsArraySink extends VulnerableCommandSink {
     ArgsArraySink() {
-      exists(SystemExecAsCmdCall exec |
-        exec.getCommandArg().mayHaveStringValue(any(VulnerableCommand cmd))
-      |
+      exists(SystemExecAsCmdCall exec | exec.getCommandArg().mayHaveStringValue(cmd) |
         this = exec.getArgList()
       )
     }

javascript/ql/src/Security/CWE-078/SecondOrderCommandInjection.ql

@@ -17,8 +17,9 @@ import javascript
 import DataFlow::PathGraph
 import semmle.javascript.security.dataflow.SecondOrderCommandInjectionQuery
 
-from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
-where cfg.hasFlowPath(source, sink)
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
+where cfg.hasFlowPath(source, sink) and sinkNode = sink.getNode()
 select sink.getNode(), source, sink,
-  "Command line argument that allows for arbitrary command execution depends on $@.",
+  "Command line argument that depends on $@ can execute an arbitrary command if " +
+    sinkNode.getVulnerableArgumentExample() + " is used with " + sinkNode.getCommand() + ".",
   source.getNode(), source.getNode().(Source).describe()

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

@@ -47,12 +47,12 @@ edges
 | second-order.js:42:31:42:46 | req.query.remote | second-order.js:42:31:42:46 | req.query.remote |
 | second-order.js:44:18:44:31 | req.query.args | second-order.js:44:18:44:31 | req.query.args |
 #select
-| second-order.js:7:33:7:38 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:7:33:7:38 | remote | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
-| second-order.js:9:29:9:34 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:9:29:9:34 | remote | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
-| second-order.js:11:33:11:38 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:11:33:11:38 | remote | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
-| second-order.js:15:19:15:24 | myArgs | second-order.js:13:18:13:31 | req.query.args | second-order.js:15:19:15:24 | myArgs | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:13:18:13:31 | req.query.args | a user-provided value |
-| second-order.js:26:35:26:40 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:26:35:26:40 | remote | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
-| second-order.js:29:19:29:32 | req.query.args | second-order.js:29:19:29:32 | req.query.args | second-order.js:29:19:29:32 | req.query.args | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:29:19:29:32 | req.query.args | a user-provided value |
-| second-order.js:40:28:40:43 | req.query.remote | second-order.js:40:28:40:43 | req.query.remote | second-order.js:40:28:40:43 | req.query.remote | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:40:28:40:43 | req.query.remote | a user-provided value |
-| second-order.js:42:31:42:46 | req.query.remote | second-order.js:42:31:42:46 | req.query.remote | second-order.js:42:31:42:46 | req.query.remote | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:42:31:42:46 | req.query.remote | a user-provided value |
-| second-order.js:44:18:44:31 | req.query.args | second-order.js:44:18:44:31 | req.query.args | second-order.js:44:18:44:31 | req.query.args | Command line argument that allows for arbitrary command execution depends on $@. | second-order.js:44:18:44:31 | req.query.args | a user-provided value |
+| second-order.js:7:33:7:38 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:7:33:7:38 | remote | Command line argument that depends on $@ can execute an arbitrary command if --upload-pack is used with git. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
+| second-order.js:9:29:9:34 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:9:29:9:34 | remote | Command line argument that depends on $@ can execute an arbitrary command if --upload-pack is used with git. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
+| second-order.js:11:33:11:38 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:11:33:11:38 | remote | Command line argument that depends on $@ can execute an arbitrary command if --upload-pack is used with git. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
+| second-order.js:15:19:15:24 | myArgs | second-order.js:13:18:13:31 | req.query.args | second-order.js:15:19:15:24 | myArgs | Command line argument that depends on $@ can execute an arbitrary command if --upload-pack is used with git. | second-order.js:13:18:13:31 | req.query.args | a user-provided value |
+| second-order.js:26:35:26:40 | remote | second-order.js:6:18:6:33 | req.query.remote | second-order.js:26:35:26:40 | remote | Command line argument that depends on $@ can execute an arbitrary command if --upload-pack is used with git. | second-order.js:6:18:6:33 | req.query.remote | a user-provided value |
+| second-order.js:29:19:29:32 | req.query.args | second-order.js:29:19:29:32 | req.query.args | second-order.js:29:19:29:32 | req.query.args | Command line argument that depends on $@ can execute an arbitrary command if --upload-pack is used with git. | second-order.js:29:19:29:32 | req.query.args | a user-provided value |
+| second-order.js:40:28:40:43 | req.query.remote | second-order.js:40:28:40:43 | req.query.remote | second-order.js:40:28:40:43 | req.query.remote | Command line argument that depends on $@ can execute an arbitrary command if --config=alias.<alias>=<command> is used with hg. | second-order.js:40:28:40:43 | req.query.remote | a user-provided value |
+| second-order.js:42:31:42:46 | req.query.remote | second-order.js:42:31:42:46 | req.query.remote | second-order.js:42:31:42:46 | req.query.remote | Command line argument that depends on $@ can execute an arbitrary command if --config=alias.<alias>=<command> is used with hg. | second-order.js:42:31:42:46 | req.query.remote | a user-provided value |
+| second-order.js:44:18:44:31 | req.query.args | second-order.js:44:18:44:31 | req.query.args | second-order.js:44:18:44:31 | req.query.args | Command line argument that depends on $@ can execute an arbitrary command if --config=alias.<alias>=<command> is used with hg. | second-order.js:44:18:44:31 | req.query.args | a user-provided value |