From b6c35dd88cb41a30b6fa1ba989ac47a357be03c4 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 16 Jun 2023 17:12:53 +0100 Subject: [PATCH] Added experimental version of Java Command Injection query, to be more sensitive to unusual code constructs --- .../CWE-078/CommandInjectionRuntimeExec.java | 9 ++ .../CWE-078/CommandInjectionRuntimeExec.md | 30 +++++ .../CWE-078/CommandInjectionRuntimeExec.qhelp | 46 +++++++ .../CWE-078/CommandInjectionRuntimeExec.ql | 33 +++++ .../CWE-078/CommandInjectionRuntimeExec.qll | 127 ++++++++++++++++++ .../CommandInjectionRuntimeExecLocal.md | 30 +++++ .../CommandInjectionRuntimeExecLocal.qhelp | 46 +++++++ .../CommandInjectionRuntimeExecLocal.ql | 34 +++++ .../CommandInjectionRuntimeExecTest.ql | 34 +++++ .../CommandInjectionRuntimeExecTestPath.ql | 34 +++++ 10 files changed, 423 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql create mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.java b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.java new file mode 100644 index 00000000000..12c7e952d99 --- /dev/null +++ b/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}); + } + } +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md new file mode 100644 index 00000000000..02222abc46e --- /dev/null +++ b/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). diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp new file mode 100644 index 00000000000..c5d7b553a67 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp @@ -0,0 +1,46 @@ + + + +

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.

+ + + + +

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.

+ + + + +

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.

+ + + + + + +
  • +OWASP: +Command Injection. +
    • +
  • SEI CERT Oracle Coding Standard for Java: + IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method.
    • + + + + + +     +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql new file mode 100644 index 00000000000..e6687aa148a --- /dev/null +++ b/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() diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll new file mode 100644 index 00000000000..ac4a9074a42 --- /dev/null +++ b/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") + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md new file mode 100644 index 00000000000..deaede379ec --- /dev/null +++ b/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). diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp new file mode 100644 index 00000000000..c8a3beba81c --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp @@ -0,0 +1,46 @@ + + + +

    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.

    + +     + + +

    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.

    + +     + + +

    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.

    + + + +     + + +
  • +OWASP: +Command Injection. +
    • +
  • SEI CERT Oracle Coding Standard for Java: + IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method.
    • + + + + + +     +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql new file mode 100644 index 00000000000..1dc990cb096 --- /dev/null +++ b/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() diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql new file mode 100644 index 00000000000..1b3f601a404 --- /dev/null +++ b/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() diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql new file mode 100644 index 00000000000..15631c86f79 --- /dev/null +++ b/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()