Added experimental version of Java Command Injection query, to be more sensitive to unusual code constructs

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.java

@@ -0,0 +1,9 @@
+class Test {
+    public static void main(String[] args) {
+        String script = System.getenv("SCRIPTNAME");
+        if (script != null) {
+            // BAD: The script to be executed by /bin/sh is controlled by the user.
+            Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
+        }
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md

@@ -0,0 +1,30 @@
+# Command Injection into Runtime.exec() with dangerous command
+Code that passes remote user input to an arugment of a call of `Runtime.exec` that executes a scripting executable will allow the user to execute malicious code.
+
+
+## Recommendation
+If possible, use hard-coded string literals to specify the command or script to run, or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
+
+If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user input string is safe before using it.
+
+
+## Example
+The following example shows code that takes a shell script that can be changed maliciously by a user, and passes it straight to the array going into `Runtime.exec` without examining it first.
+
+
+```java
+class Test {
+    public static void main(String[] args) {
+        String script = System.getenv("SCRIPTNAME");
+        if (script != null) {
+            // BAD: The script to be executed by /bin/sh is controlled by the user.
+            Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
+        }
+    }
+}
+```
+
+## References
+* OWASP: [Command Injection](https://www.owasp.org/index.php/Command_Injection).
+* SEI CERT Oracle Coding Standard for Java: [IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method](https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method).
+* Common Weakness Enumeration: [CWE-78](https://cwe.mitre.org/data/definitions/78.html).

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp

@@ -0,0 +1,46 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Code that passes remote user input to an arugment of a call of <code>Runtime.exec</code> that
+executes a scripting executable will allow the user to execute malicious code.</p>
+
+</overview>
+<recommendation>
+
+<p>If possible, use hard-coded string literals to specify the command or script to run,
+or library to load. Instead of passing the user input directly to the
+process or library function, examine the user input and then choose
+among hard-coded string literals.</p>
+
+<p>If the applicable libraries or commands cannot be determined at
+compile time, then add code to verify that the user input string is
+safe before using it.</p>
+
+</recommendation>
+<example>
+
+<p>The following example shows code that takes a shell script that can be changed
+maliciously by a user, and passes it straight to the array going into <code>Runtime.exec</code>
+without examining it first.</p>
+
+<sample src="CommandInjectionRuntimeExec.java" />
+
+</example>
+<references>
+
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
+</li>
+<li>SEI CERT Oracle Coding Standard for Java:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method">IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method</a>.</li>
+
+
+
+<!--  LocalWords:  CWE untrusted unsanitized Runtime
+ -->
+
+</references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql

@@ -0,0 +1,33 @@
+/**
+ * @name Command Injection into Runtime.exec() with dangerous command
+ * @description High sensitvity and precision version of java/command-line-injection, designed to find more cases of command injection in rare cases that the default query does not find
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 6.1
+ * @precision high
+ * @id java/command-line-injection-extra
+ * @tags security
+ *       external/cwe/cwe-078
+ */
+
+
+import DataFlow::PathGraph
+import CommandInjectionRuntimeExec
+
+class RemoteSource extends Source { RemoteSource() { this instanceof RemoteFlowSource } }
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
+where call.getMethod() instanceof RuntimeExecMethod
+// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
+and (
+    confCmd.hasFlow(sourceCmd, sinkCmd)
+    and sinkCmd.asExpr() = call.getArgument(0)
+)
+// it is tainted by untrusted user input
+and (
+    conf.hasFlow(source.getNode(), sink.getNode())
+    and sink.getNode().asExpr() = call.getArgument(0)
+)
+select sink, source, sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
+    sourceCmd, sourceCmd.toString(),
+    source.getNode(), source.toString()

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll

@@ -0,0 +1,127 @@
+import java
+import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
+import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+
+
+// a static string of an unsafe executable tainting arg 0 of Runtime.exec()
+class ExecTaintConfiguration extends TaintTracking::Configuration {
+    ExecTaintConfiguration() { this = "ExecTaintConfiguration" }
+
+    override
+    predicate
+    isSource(DataFlow::Node source) {
+        source.asExpr() instanceof StringLiteral
+        and source.asExpr().(StringLiteral).getValue() instanceof UnSafeExecutable
+    }
+
+    override
+    predicate
+    isSink(DataFlow::Node sink) {
+        exists(RuntimeExecMethod method, MethodAccess call |
+            call.getMethod() = method
+            and sink.asExpr() = call.getArgument(0)
+            and sink.asExpr().getType() instanceof Array
+        )
+    }
+
+    override
+    predicate
+    isSanitizer(DataFlow::Node node) {
+        node.asExpr().getFile().isSourceFile() and
+        (
+            node instanceof AssignToNonZeroIndex
+            or node instanceof ArrayInitAtNonZeroIndex
+            or node instanceof StreamConcatAtNonZeroIndex
+            or node.getType() instanceof PrimitiveType
+            or node.getType() instanceof BoxedType
+        )
+    }
+}
+
+abstract class Source extends DataFlow::Node {
+    Source() {
+        this = this
+    }
+}
+
+
+// taint flow from user data to args of the command
+class ExecTaintConfiguration2 extends TaintTracking::Configuration {
+    ExecTaintConfiguration2() { this = "ExecTaintConfiguration2" }
+
+    override
+    predicate
+    isSource(DataFlow::Node source) {
+        source instanceof Source
+    }
+
+    override
+    predicate
+    isSink(DataFlow::Node sink) {
+        exists(RuntimeExecMethod method, MethodAccess call, int index |
+            call.getMethod() = method
+            and sink.asExpr() = call.getArgument(index)
+            and sink.asExpr().getType() instanceof Array
+        )
+    }
+
+    override
+    predicate
+    isSanitizer(DataFlow::Node node) {
+        node.asExpr().getFile().isSourceFile() and
+        (
+            node.getType() instanceof PrimitiveType
+            or node.getType() instanceof BoxedType
+        )
+    }
+}
+
+
+// array[3] = node
+class AssignToNonZeroIndex extends DataFlow::Node {
+    AssignExpr assign;
+    ArrayAccess access;
+
+    AssignToNonZeroIndex() {
+        assign.getDest() = access
+        and access.getIndexExpr().(IntegerLiteral).getValue() != "0"
+        and assign.getSource() = this.asExpr()
+    }
+}
+
+
+// String[] array = {"a", "b, "c"};
+class ArrayInitAtNonZeroIndex extends DataFlow::Node {
+    ArrayInit init;
+    int index;
+
+    ArrayInitAtNonZeroIndex() {
+        init.getInit(index) = this.asExpr()
+        and index != 0
+    }
+}
+
+// Stream.concat(Arrays.stream(array_1), Arrays.stream(array_2))
+class StreamConcatAtNonZeroIndex extends DataFlow::Node {
+    MethodAccess call;
+    int index;
+
+    StreamConcatAtNonZeroIndex() {
+        call.getMethod().getQualifiedName() = "java.util.stream.Stream.concat"
+        and call.getArgument(index) = this.asExpr()
+        and index != 0
+    }
+}
+
+
+// allow list of executables that execute their arguments
+// TODO: extend with data extensions
+class UnSafeExecutable extends string {
+    bindingset[this]
+    UnSafeExecutable() {
+        this.regexpMatch("^(|.*/)([a-z]*sh|javac?|python.*|perl|[Pp]ower[Ss]hell|php|node|deno|bun|ruby|osascript|cmd|Rscript|groovy)(\\.exe)?$")
+        and not this.matches("netsh.exe")
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md

@@ -0,0 +1,30 @@
+# Command Injection into Runtime.exec() with dangerous command
+Code that passes local user input to an arugment of a call of `Runtime.exec` that executes a scripting executable will allow the user to execute malicious code.
+
+
+## Recommendation
+If possible, use hard-coded string literals to specify the command or script to run, or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
+
+If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user input string is safe before using it.
+
+
+## Example
+The following example shows code that takes a shell script that can be changed maliciously by a user, and passes it straight to the array going into `Runtime.exec` without examining it first.
+
+
+```java
+class Test {
+    public static void main(String[] args) {
+        String script = System.getenv("SCRIPTNAME");
+        if (script != null) {
+            // BAD: The script to be executed by /bin/sh is controlled by the user.
+            Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
+        }
+    }
+}
+```
+
+## References
+* OWASP: [Command Injection](https://www.owasp.org/index.php/Command_Injection).
+* SEI CERT Oracle Coding Standard for Java: [IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method](https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method).
+* Common Weakness Enumeration: [CWE-78](https://cwe.mitre.org/data/definitions/78.html).

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp

@@ -0,0 +1,46 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Code that passes local user input to an arugment of a call of <code>Runtime.exec</code> that
+executes a scripting executable will allow the user to execute malicious code.</p>
+
+</overview>
+<recommendation>
+
+<p>If possible, use hard-coded string literals to specify the command or script to run,
+or library to load. Instead of passing the user input directly to the
+process or library function, examine the user input and then choose
+among hard-coded string literals.</p>
+
+<p>If the applicable libraries or commands cannot be determined at
+compile time, then add code to verify that the user input string is
+safe before using it.</p>
+
+</recommendation>
+<example>
+
+<p>The following example shows code that takes a shell script that can be changed
+maliciously by a user, and passes it straight to the array going into <code>Runtime.exec</code>
+without examining it first.</p>
+
+<sample src="CommandInjectionRuntimeExec.java" />
+
+</example>
+<references>
+
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
+</li>
+<li>SEI CERT Oracle Coding Standard for Java:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method">IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method</a>.</li>
+
+
+
+<!--  LocalWords:  CWE untrusted unsanitized local Runtime
+ -->
+
+</references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql

@@ -0,0 +1,34 @@
+/**
+ * @name Command Injection into Runtime.exec() with dangerous command
+ * @description High sensitvity and precision version of java/command-line-injection, designed to find more cases of command injection in rare cases that the default query does not find
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 6.1
+ * @precision high
+ * @id java/command-line-injection-extra-local
+ * @tags security
+ *       local
+ *       external/cwe/cwe-078
+ */
+
+
+import DataFlow::PathGraph
+import CommandInjectionRuntimeExec
+
+class LocalSource extends Source { LocalSource() { this instanceof LocalUserInput } }
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
+where call.getMethod() instanceof RuntimeExecMethod
+// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
+and (
+    confCmd.hasFlow(sourceCmd, sinkCmd)
+    and sinkCmd.asExpr() = call.getArgument(0)
+)
+// it is tainted by untrusted user input
+and (
+    conf.hasFlow(source.getNode(), sink.getNode())
+    and sink.getNode().asExpr() = call.getArgument(0)
+)
+select sink, source, sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
+    sourceCmd, sourceCmd.toString(),
+    source.getNode(), source.toString()

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql

@@ -0,0 +1,34 @@
+/**
+ * @name Command Injection into Runtime.exec() with dangerous command
+ * @description Testing query. High sensitvity and precision version of java/command-line-injection, designed to find more cases of command injection in rare cases that the default query does not find
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 6.1
+ * @precision high
+ * @id java/command-line-injection-extra-test
+ * @tags testing
+ *       test
+ *       security
+ *       external/cwe/cwe-078
+ */
+
+
+import CommandInjectionRuntimeExec
+
+class DataSource extends Source { DataSource() { this instanceof RemoteFlowSource or this instanceof LocalUserInput } }
+
+from DataFlow::Node source, DataFlow::Node sink, ExecTaintConfiguration2 conf, MethodAccess call, int index, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
+where call.getMethod() instanceof RuntimeExecMethod
+// this is a command-accepting call to exec, e.g. exec("/bin/sh", ...)
+and (
+    confCmd.hasFlow(sourceCmd, sinkCmd)
+    and sinkCmd.asExpr() = call.getArgument(0)
+)
+// it is tainted by untrusted user input
+and (
+    conf.hasFlow(source, sink)
+    and sink.asExpr() = call.getArgument(index)
+)
+select sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
+    sourceCmd, sourceCmd.toString(),
+    source, source.toString()

java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql

@@ -0,0 +1,34 @@
+/**
+ * @name Command Injection into Runtime.exec() with dangerous command
+ * @description Testing query. High sensitvity and precision version of java/command-line-injection, designed to find more cases of command injection in rare cases that the default query does not find
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 6.1
+ * @precision high
+ * @id java/command-line-injection-extra-test-path
+ * @tags testing
+ *       test
+ *       security
+ *       external/cwe/cwe-078
+ */
+
+import DataFlow::PathGraph
+import CommandInjectionRuntimeExec
+
+class DataSource extends Source { DataSource() { this instanceof RemoteFlowSource or this instanceof LocalUserInput } }
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
+where call.getMethod() instanceof RuntimeExecMethod
+// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
+and (
+    confCmd.hasFlow(sourceCmd, sinkCmd)
+    and sinkCmd.asExpr() = call.getArgument(0)
+)
+// it is tainted by untrusted user input
+and (
+    conf.hasFlow(source.getNode(), sink.getNode())
+    and sink.getNode().asExpr() = call.getArgument(0)
+)
+select sink, source, sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
+    sourceCmd, sourceCmd.toString(),
+    source.getNode(), source.toString()