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 01/15] 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 000000000000..12c7e952d99e --- /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 000000000000..02222abc46ed --- /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 000000000000..c5d7b553a672 --- /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 000000000000..e6687aa148a4 --- /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 000000000000..ac4a9074a423 --- /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 000000000000..deaede379ec3 --- /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 000000000000..c8a3beba81c0 --- /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 000000000000..1dc990cb096a --- /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 000000000000..1b3f601a4047 --- /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 000000000000..15631c86f794 --- /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() From 55eeb0030908e85b2b28ad287662c46ac067e556 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 16 Jun 2023 17:27:01 +0100 Subject: [PATCH 02/15] Added experimental tag --- .../Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql | 1 + .../Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql | 1 + .../Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql | 1 + .../Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql | 1 + 4 files changed, 4 insertions(+) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index e6687aa148a4..9c770b25fad0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -7,6 +7,7 @@ * @precision high * @id java/command-line-injection-extra * @tags security + * experimental * external/cwe/cwe-078 */ diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index 1dc990cb096a..529c1a4a921d 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -7,6 +7,7 @@ * @precision high * @id java/command-line-injection-extra-local * @tags security + * experimental * local * external/cwe/cwe-078 */ diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql index 1b3f601a4047..61ded91f7541 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql @@ -8,6 +8,7 @@ * @id java/command-line-injection-extra-test * @tags testing * test + * experimental * security * external/cwe/cwe-078 */ diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql index 15631c86f794..e379b8477663 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql @@ -8,6 +8,7 @@ * @id java/command-line-injection-extra-test-path * @tags testing * test + * experimental * security * external/cwe/cwe-078 */ From 8c73fbeabe4511acb5709ce50e4df54b879a1a12 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 16 Jun 2023 17:33:21 +0100 Subject: [PATCH 03/15] Formatted --- .../CWE-078/CommandInjectionRuntimeExec.qll | 165 ++++++++---------- .../CommandInjectionRuntimeExecLocal.ql | 39 +++-- .../CommandInjectionRuntimeExecTest.ql | 38 ++-- .../CommandInjectionRuntimeExecTestPath.ql | 38 ++-- 4 files changed, 137 insertions(+), 143 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll index ac4a9074a423..c0abb621c465 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -4,124 +4,103 @@ 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 - ) - } + 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 - } + 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 - ) - } + 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() - } + 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; + ArrayInit init; + int index; - ArrayInitAtNonZeroIndex() { - init.getInit(index) = this.asExpr() - and index != 0 - } + 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 - } + 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") - } + 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.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index 529c1a4a921d..94a8fba23fda 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -12,24 +12,29 @@ * external/cwe/cwe-078 */ - import DataFlow::PathGraph import CommandInjectionRuntimeExec -class LocalSource extends Source { LocalSource() { this instanceof LocalUserInput } } +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() +from + DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, + MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, + ExecTaintConfiguration confCmd +where + call.getMethod() instanceof RuntimeExecMethod and + // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) + ( + confCmd.hasFlow(sourceCmd, sinkCmd) and + sinkCmd.asExpr() = call.getArgument(0) + ) and + // it is tainted by untrusted user input + ( + 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 index 61ded91f7541..f0758e0d2a58 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql @@ -13,23 +13,27 @@ * external/cwe/cwe-078 */ - import CommandInjectionRuntimeExec -class DataSource extends Source { DataSource() { this instanceof RemoteFlowSource or this instanceof LocalUserInput } } +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() +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 and + // this is a command-accepting call to exec, e.g. exec("/bin/sh", ...) + ( + confCmd.hasFlow(sourceCmd, sinkCmd) and + sinkCmd.asExpr() = call.getArgument(0) + ) and + // it is tainted by untrusted user input + ( + 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 index e379b8477663..33f6a0c3c301 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql @@ -16,20 +16,26 @@ import DataFlow::PathGraph import CommandInjectionRuntimeExec -class DataSource extends Source { DataSource() { this instanceof RemoteFlowSource or this instanceof LocalUserInput } } +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() +from + DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, + MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, + ExecTaintConfiguration confCmd +where + call.getMethod() instanceof RuntimeExecMethod and + // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) + ( + confCmd.hasFlow(sourceCmd, sinkCmd) and + sinkCmd.asExpr() = call.getArgument(0) + ) and + // it is tainted by untrusted user input + ( + 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() From 7c235e3786c05e62ace5cc70bb9fd2449c505fb3 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 19 Jun 2023 11:41:23 +0100 Subject: [PATCH 04/15] Fixed linting issues. Will not fix instanceof, that is necessary --- .../CWE-078/CommandInjectionRuntimeExec.qll | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll index c0abb621c465..5c60ea15038f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -44,9 +44,9 @@ class ExecTaintConfiguration2 extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof Source } override predicate isSink(DataFlow::Node sink) { - exists(RuntimeExecMethod method, MethodAccess call, int index | + exists(RuntimeExecMethod method, MethodAccess call | call.getMethod() = method and - sink.asExpr() = call.getArgument(index) and + sink.asExpr() = call.getArgument(_) and sink.asExpr().getType() instanceof Array ) } @@ -62,36 +62,33 @@ class ExecTaintConfiguration2 extends TaintTracking::Configuration { // 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() + exists(AssignExpr assign, ArrayAccess access | + 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 + exists(ArrayInit init, int index | + 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 + exists(MethodAccess call, int index | + call.getMethod().getQualifiedName() = "java.util.stream.Stream.concat" and + call.getArgument(index) = this.asExpr() and + index != 0 + ) } } From 1a108fb1c9565101e8a577793bcc9c7b3c492b32 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 19 Jun 2023 11:46:08 +0100 Subject: [PATCH 05/15] Changed to for constant string --- .../Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll index 5c60ea15038f..9b3312e20538 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -98,6 +98,6 @@ 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") + not this = "netsh.exe" } } From 2112d73a6a7bdf1bac8c6feb4ddee178a5df9587 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 19 Jun 2023 11:50:54 +0100 Subject: [PATCH 06/15] Autoformat --- .../CWE/CWE-078/CommandInjectionRuntimeExec.qll | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll index 9b3312e20538..abc161563d8c 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -64,9 +64,9 @@ class ExecTaintConfiguration2 extends TaintTracking::Configuration { class AssignToNonZeroIndex extends DataFlow::Node { AssignToNonZeroIndex() { exists(AssignExpr assign, ArrayAccess access | - assign.getDest() = access and - access.getIndexExpr().(IntegerLiteral).getValue() != "0" and - assign.getSource() = this.asExpr() + assign.getDest() = access and + access.getIndexExpr().(IntegerLiteral).getValue() != "0" and + assign.getSource() = this.asExpr() ) } } @@ -75,8 +75,8 @@ class AssignToNonZeroIndex extends DataFlow::Node { class ArrayInitAtNonZeroIndex extends DataFlow::Node { ArrayInitAtNonZeroIndex() { exists(ArrayInit init, int index | - init.getInit(index) = this.asExpr() and - index != 0 + init.getInit(index) = this.asExpr() and + index != 0 ) } } @@ -85,9 +85,9 @@ class ArrayInitAtNonZeroIndex extends DataFlow::Node { class StreamConcatAtNonZeroIndex extends DataFlow::Node { StreamConcatAtNonZeroIndex() { exists(MethodAccess call, int index | - call.getMethod().getQualifiedName() = "java.util.stream.Stream.concat" and - call.getArgument(index) = this.asExpr() and - index != 0 + call.getMethod().getQualifiedName() = "java.util.stream.Stream.concat" and + call.getArgument(index) = this.asExpr() and + index != 0 ) } } From 8c9ccab9c993efe4a5139cc2cc8a02bb29db5ef0 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 19 Jun 2023 11:53:53 +0100 Subject: [PATCH 07/15] Autoformat --- .../CWE-078/CommandInjectionRuntimeExec.ql | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index 9c770b25fad0..edfc5c911d1d 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -11,24 +11,29 @@ * external/cwe/cwe-078 */ - import DataFlow::PathGraph import CommandInjectionRuntimeExec -class RemoteSource extends Source { RemoteSource() { this instanceof RemoteFlowSource } } +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() +from + DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, + MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, + ExecTaintConfiguration confCmd +where + call.getMethod() instanceof RuntimeExecMethod and + // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) + ( + confCmd.hasFlow(sourceCmd, sinkCmd) and + sinkCmd.asExpr() = call.getArgument(0) + ) and + // it is tainted by untrusted user input + ( + 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() From 23bf8470ce9450cbd1d8e5cbe78d867be012fd4c Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 19 Jun 2023 17:29:17 +0100 Subject: [PATCH 08/15] Removed .md and made class change --- .../CWE-078/CommandInjectionRuntimeExec.md | 30 ------------------- .../CWE-078/CommandInjectionRuntimeExec.ql | 4 +-- .../CommandInjectionRuntimeExecLocal.md | 30 ------------------- .../CommandInjectionRuntimeExecLocal.ql | 4 +-- 4 files changed, 2 insertions(+), 66 deletions(-) delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md deleted file mode 100644 index 02222abc46ed..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.md +++ /dev/null @@ -1,30 +0,0 @@ -# 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.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index edfc5c911d1d..1350e9061f52 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -14,9 +14,7 @@ import DataFlow::PathGraph import CommandInjectionRuntimeExec -class RemoteSource extends Source { - RemoteSource() { this instanceof RemoteFlowSource } -} +class RemoteSource extends Source instanceof RemoteFlowSource {} from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md deleted file mode 100644 index deaede379ec3..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.md +++ /dev/null @@ -1,30 +0,0 @@ -# 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.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index 94a8fba23fda..fc2d4630e75a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -15,9 +15,7 @@ import DataFlow::PathGraph import CommandInjectionRuntimeExec -class LocalSource extends Source { - LocalSource() { this instanceof LocalUserInput } -} +class LocalSource extends Source instanceof LocalUserInput {} from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, From 01798f63f8f587fc2e923b84e7fb57da4ded9546 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Wed, 28 Jun 2023 17:14:39 +0100 Subject: [PATCH 09/15] Switched to new dataflow and added a test (but it doesn't produce results yet) --- .../CWE-078/CommandInjectionRuntimeExec.ql | 21 ++---- .../CWE-078/CommandInjectionRuntimeExec.qll | 73 ++++++++++++++++++- .../CommandInjectionRuntimeExecLocal.ql | 21 ++---- .../CommandInjectionRuntimeExecTest.ql | 39 ---------- .../CommandInjectionRuntimeExecTestPath.ql | 41 ----------- .../CommandInjectionRuntimeExecLocal.qlref | 1 + .../security/CWE-078/RuntimeExecTest.java | 42 +++++++++++ 7 files changed, 123 insertions(+), 115 deletions(-) delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql create mode 100644 java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.qlref create mode 100644 java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index 1350e9061f52..f92d16e5c2bd 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -11,27 +11,16 @@ * external/cwe/cwe-078 */ -import DataFlow::PathGraph import CommandInjectionRuntimeExec +import ExecUserFlow::PathGraph -class RemoteSource extends Source instanceof RemoteFlowSource {} +class RemoteSource extends Source instanceof RemoteFlowSource { } from - DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, - MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, - ExecTaintConfiguration confCmd + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, + MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd where - call.getMethod() instanceof RuntimeExecMethod and - // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) - ( - confCmd.hasFlow(sourceCmd, sinkCmd) and - sinkCmd.asExpr() = call.getArgument(0) - ) and - // it is tainted by untrusted user input - ( - conf.hasFlow(source.getNode(), sink.getNode()) and - sink.getNode().asExpr() = call.getArgument(0) - ) + callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) 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 index abc161563d8c..2c39685c563c 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -1,11 +1,10 @@ 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 { +deprecated class ExecTaintConfiguration extends TaintTracking::Configuration { ExecTaintConfiguration() { this = "ExecTaintConfiguration" } override predicate isSource(DataFlow::Node source) { @@ -33,12 +32,41 @@ class ExecTaintConfiguration extends TaintTracking::Configuration { } } +module ExecCmdFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof StringLiteral and + source.asExpr().(StringLiteral).getValue() instanceof UnSafeExecutable + } + + 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 + ) + } + + predicate isBarrier(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 + ) + } +} + +/** Tracks flow of unvalidated user input that is used in Runtime.Exec */ +module ExecCmdFlow = TaintTracking::Global; + abstract class Source extends DataFlow::Node { Source() { this = this } } // taint flow from user data to args of the command -class ExecTaintConfiguration2 extends TaintTracking::Configuration { +deprecated class ExecTaintConfiguration2 extends TaintTracking::Configuration { ExecTaintConfiguration2() { this = "ExecTaintConfiguration2" } override predicate isSource(DataFlow::Node source) { source instanceof Source } @@ -60,6 +88,31 @@ class ExecTaintConfiguration2 extends TaintTracking::Configuration { } } +module ExecUserFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source instanceof Source + } + + predicate isSink(DataFlow::Node sink) { + exists(RuntimeExecMethod method, MethodAccess call | + call.getMethod() = method and + sink.asExpr() = call.getArgument(_) and + sink.asExpr().getType() instanceof Array + ) + } + + predicate isBarrier(DataFlow::Node node) { + node.asExpr().getFile().isSourceFile() and + ( + node.getType() instanceof PrimitiveType or + node.getType() instanceof BoxedType + ) + } +} + +/** Tracks flow of unvalidated user input that is used in Runtime.Exec */ +module ExecUserFlow = TaintTracking::Global; + // array[3] = node class AssignToNonZeroIndex extends DataFlow::Node { AssignToNonZeroIndex() { @@ -101,3 +154,17 @@ class UnSafeExecutable extends string { not this = "netsh.exe" } } + +predicate callIsTaintedByUserInputAndDangerousCommand(MethodAccess call, ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd) { + call.getMethod() instanceof RuntimeExecMethod and + // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) + ( + ExecCmdFlow::flow(sourceCmd, sinkCmd) and + sinkCmd.asExpr() = call.getArgument(0) + ) and + // it is tainted by untrusted user input + ( + ExecUserFlow::flowPath(source, sink) and + sink.getNode().asExpr() = call.getArgument(0) + ) +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index fc2d4630e75a..505743b3949f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -12,27 +12,16 @@ * external/cwe/cwe-078 */ -import DataFlow::PathGraph import CommandInjectionRuntimeExec +import ExecUserFlow::PathGraph -class LocalSource extends Source instanceof LocalUserInput {} +class LocalSource extends Source instanceof LocalUserInput { } from - DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, - MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, - ExecTaintConfiguration confCmd + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, + MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd where - call.getMethod() instanceof RuntimeExecMethod and - // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) - ( - confCmd.hasFlow(sourceCmd, sinkCmd) and - sinkCmd.asExpr() = call.getArgument(0) - ) and - // it is tainted by untrusted user input - ( - conf.hasFlow(source.getNode(), sink.getNode()) and - sink.getNode().asExpr() = call.getArgument(0) - ) + callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) 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 deleted file mode 100644 index f0758e0d2a58..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTest.ql +++ /dev/null @@ -1,39 +0,0 @@ -/** - * @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 - * experimental - * 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 and - // this is a command-accepting call to exec, e.g. exec("/bin/sh", ...) - ( - confCmd.hasFlow(sourceCmd, sinkCmd) and - sinkCmd.asExpr() = call.getArgument(0) - ) and - // it is tainted by untrusted user input - ( - 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 deleted file mode 100644 index 33f6a0c3c301..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecTestPath.ql +++ /dev/null @@ -1,41 +0,0 @@ -/** - * @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 - * experimental - * 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 and - // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) - ( - confCmd.hasFlow(sourceCmd, sinkCmd) and - sinkCmd.asExpr() = call.getArgument(0) - ) and - // it is tainted by untrusted user input - ( - 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/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.qlref b/java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.qlref new file mode 100644 index 000000000000..6916c533a167 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java b/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java new file mode 100644 index 000000000000..061d28a60a61 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java @@ -0,0 +1,42 @@ +/* Tests for command injection query + * + * This is suitable for testing static analysis tools, as long as they treat local input as an attack surface (which can be prone to false positives) + * + * (C) Copyright GitHub, 2023 + * + */ + +import java.util.stream.Stream; +import java.io.IOException; +import java.util.Arrays; + +public class RuntimeExecTest { + public static void test(String[] args) { + System.out.println("Command injection test"); + + try { + // 1. array literal + String[] commandArray1 = new String[]{"/bin/sh", args[2], args[3], args[4]}; + Runtime.getRuntime().exec(commandArray1); + + // 2. array assignment after it is created + String[] commandArray2 = new String[4]; + commandArray2[0] = "/bin/sh"; + commandArray2[1] = args[2]; + commandArray2[2] = args[3]; + commandArray2[3] = args[4]; + Runtime.getRuntime().exec(commandArray2); + + // 3. Stream concatenation + Runtime.getRuntime().exec( + Stream.concat( + Arrays.stream(new String[]{"/bin/sh"}), + Arrays.stream(new String[]{args[2], args[3], args[4]}) + ).toArray(String[]::new) + ); + + } catch (Exception e) { + System.err.println("ERROR: " + e.getMessage()); + } + } +} From 8dbb0a51c08d26a78c7dd3c7860a2034a28e9855 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Thu, 29 Jun 2023 09:47:03 +0100 Subject: [PATCH 10/15] Rewrote tests to work --- .../security/CWE-078/RuntimeExecTest.java | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java b/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java index 061d28a60a61..203c3855c87d 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java +++ b/java/ql/test/experimental/query-tests/security/CWE-078/RuntimeExecTest.java @@ -11,32 +11,37 @@ import java.util.Arrays; public class RuntimeExecTest { - public static void test(String[] args) { + public static void test() { System.out.println("Command injection test"); - try { - // 1. array literal - String[] commandArray1 = new String[]{"/bin/sh", args[2], args[3], args[4]}; - Runtime.getRuntime().exec(commandArray1); - - // 2. array assignment after it is created - String[] commandArray2 = new String[4]; - commandArray2[0] = "/bin/sh"; - commandArray2[1] = args[2]; - commandArray2[2] = args[3]; - commandArray2[3] = args[4]; - Runtime.getRuntime().exec(commandArray2); - - // 3. Stream concatenation - Runtime.getRuntime().exec( - Stream.concat( - Arrays.stream(new String[]{"/bin/sh"}), - Arrays.stream(new String[]{args[2], args[3], args[4]}) - ).toArray(String[]::new) - ); - - } catch (Exception e) { - System.err.println("ERROR: " + e.getMessage()); + String script = System.getenv("SCRIPTNAME"); + + if (script != null) { + try { + // 1. array literal in the args + Runtime.getRuntime().exec(new String[]{"/bin/sh", script}); + + // 2. array literal with dataflow + String[] commandArray1 = new String[]{"/bin/sh", script}; + Runtime.getRuntime().exec(commandArray1); + + // 3. array assignment after it is created + String[] commandArray2 = new String[4]; + commandArray2[0] = "/bin/sh"; + commandArray2[1] = script; + Runtime.getRuntime().exec(commandArray2); + + // 4. Stream concatenation + Runtime.getRuntime().exec( + Stream.concat( + Arrays.stream(new String[]{"/bin/sh"}), + Arrays.stream(new String[]{script}) + ).toArray(String[]::new) + ); + + } catch (Exception e) { + System.err.println("ERROR: " + e.getMessage()); + } } } } From b5d08ade59d3a9d0b873a29514a35623b007c574 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 1 Aug 2023 09:35:25 +0200 Subject: [PATCH 11/15] Formatting --- .../CWE/CWE-078/CommandInjectionRuntimeExec.ql | 7 +++---- .../CWE/CWE-078/CommandInjectionRuntimeExec.qll | 11 ++++++----- .../CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql | 7 +++---- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index f92d16e5c2bd..5829eee99744 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -17,10 +17,9 @@ import ExecUserFlow::PathGraph class RemoteSource extends Source instanceof RemoteFlowSource { } from - ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, - MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd -where - callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, MethodAccess call, + DataFlow::Node sourceCmd, DataFlow::Node sinkCmd +where callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) 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 index 2c39685c563c..e815ed5758d4 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -89,9 +89,7 @@ deprecated class ExecTaintConfiguration2 extends TaintTracking::Configuration { } module ExecUserFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source instanceof Source - } + predicate isSource(DataFlow::Node source) { source instanceof Source } predicate isSink(DataFlow::Node sink) { exists(RuntimeExecMethod method, MethodAccess call | @@ -155,7 +153,10 @@ class UnSafeExecutable extends string { } } -predicate callIsTaintedByUserInputAndDangerousCommand(MethodAccess call, ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd) { +predicate callIsTaintedByUserInputAndDangerousCommand( + MethodAccess call, ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, + DataFlow::Node sourceCmd, DataFlow::Node sinkCmd +) { call.getMethod() instanceof RuntimeExecMethod and // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) ( @@ -167,4 +168,4 @@ predicate callIsTaintedByUserInputAndDangerousCommand(MethodAccess call, ExecUse ExecUserFlow::flowPath(source, sink) and sink.getNode().asExpr() = call.getArgument(0) ) -} \ No newline at end of file +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index 505743b3949f..ba6571530604 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -18,10 +18,9 @@ import ExecUserFlow::PathGraph class LocalSource extends Source instanceof LocalUserInput { } from - ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, - MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd -where - callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, MethodAccess call, + DataFlow::Node sourceCmd, DataFlow::Node sinkCmd +where callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) 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() From 36587105787c6c2a25ba5247654e63c5b9fbe8df Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Thu, 29 Jun 2023 09:55:11 +0100 Subject: [PATCH 12/15] Fixed formatting, committed expected test results --- .../CommandInjectionRuntimeExecLocal.expected | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.expected diff --git a/java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.expected b/java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.expected new file mode 100644 index 000000000000..0baff0f6f218 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.expected @@ -0,0 +1,41 @@ +edges +| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:22:67:22:72 | script : String | +| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:25:66:25:71 | script : String | +| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:31:36:31:41 | script : String | +| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:38:52:38:57 | script : String | +| RuntimeExecTest.java:22:43:22:73 | {...} : String[] [[]] : String | RuntimeExecTest.java:22:43:22:73 | new String[] | +| RuntimeExecTest.java:22:67:22:72 | script : String | RuntimeExecTest.java:22:43:22:73 | {...} : String[] [[]] : String | +| RuntimeExecTest.java:25:42:25:72 | {...} : String[] [[]] : String | RuntimeExecTest.java:26:43:26:55 | commandArray1 | +| RuntimeExecTest.java:25:66:25:71 | script : String | RuntimeExecTest.java:25:42:25:72 | {...} : String[] [[]] : String | +| RuntimeExecTest.java:31:17:31:29 | commandArray2 [post update] : String[] [[]] : String | RuntimeExecTest.java:32:43:32:55 | commandArray2 | +| RuntimeExecTest.java:31:36:31:41 | script : String | RuntimeExecTest.java:31:17:31:29 | commandArray2 [post update] : String[] [[]] : String | +| RuntimeExecTest.java:36:21:39:21 | concat(...) : Stream [] : String | RuntimeExecTest.java:36:21:39:44 | toArray(...) : String[] [[]] : String | +| RuntimeExecTest.java:36:21:39:44 | toArray(...) : String[] [[]] : String | RuntimeExecTest.java:36:21:39:44 | toArray(...) | +| RuntimeExecTest.java:38:25:38:59 | stream(...) : Stream [] : String | RuntimeExecTest.java:36:21:39:21 | concat(...) : Stream [] : String | +| RuntimeExecTest.java:38:39:38:58 | new String[] : String[] [[]] : String | RuntimeExecTest.java:38:25:38:59 | stream(...) : Stream [] : String | +| RuntimeExecTest.java:38:39:38:58 | {...} : String[] [[]] : String | RuntimeExecTest.java:38:39:38:58 | new String[] : String[] [[]] : String | +| RuntimeExecTest.java:38:52:38:57 | script : String | RuntimeExecTest.java:38:39:38:58 | {...} : String[] [[]] : String | +nodes +| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | semmle.label | getenv(...) : String | +| RuntimeExecTest.java:22:43:22:73 | new String[] | semmle.label | new String[] | +| RuntimeExecTest.java:22:43:22:73 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String | +| RuntimeExecTest.java:22:67:22:72 | script : String | semmle.label | script : String | +| RuntimeExecTest.java:25:42:25:72 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String | +| RuntimeExecTest.java:25:66:25:71 | script : String | semmle.label | script : String | +| RuntimeExecTest.java:26:43:26:55 | commandArray1 | semmle.label | commandArray1 | +| RuntimeExecTest.java:31:17:31:29 | commandArray2 [post update] : String[] [[]] : String | semmle.label | commandArray2 [post update] : String[] [[]] : String | +| RuntimeExecTest.java:31:36:31:41 | script : String | semmle.label | script : String | +| RuntimeExecTest.java:32:43:32:55 | commandArray2 | semmle.label | commandArray2 | +| RuntimeExecTest.java:36:21:39:21 | concat(...) : Stream [] : String | semmle.label | concat(...) : Stream [] : String | +| RuntimeExecTest.java:36:21:39:44 | toArray(...) | semmle.label | toArray(...) | +| RuntimeExecTest.java:36:21:39:44 | toArray(...) : String[] [[]] : String | semmle.label | toArray(...) : String[] [[]] : String | +| RuntimeExecTest.java:38:25:38:59 | stream(...) : Stream [] : String | semmle.label | stream(...) : Stream [] : String | +| RuntimeExecTest.java:38:39:38:58 | new String[] : String[] [[]] : String | semmle.label | new String[] : String[] [[]] : String | +| RuntimeExecTest.java:38:39:38:58 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String | +| RuntimeExecTest.java:38:52:38:57 | script : String | semmle.label | script : String | +subpaths +#select +| RuntimeExecTest.java:22:43:22:73 | new String[] | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:22:43:22:73 | new String[] | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:22:56:22:64 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String | +| RuntimeExecTest.java:26:43:26:55 | commandArray1 | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:26:43:26:55 | commandArray1 | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:25:55:25:63 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String | +| RuntimeExecTest.java:32:43:32:55 | commandArray2 | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:32:43:32:55 | commandArray2 | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:30:36:30:44 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String | +| RuntimeExecTest.java:36:21:39:44 | toArray(...) | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:36:21:39:44 | toArray(...) | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:37:52:37:60 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String | From fc7f8409be2646713982fe391e2cad6ab3653fee Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 31 Jul 2023 19:09:47 +0100 Subject: [PATCH 13/15] Fix up for code review --- .../CWE-078/CommandInjectionRuntimeExec.ql | 8 +- .../CWE-078/CommandInjectionRuntimeExec.qll | 107 ++++-------------- .../CommandInjectionRuntimeExecLocal.ql | 8 +- 3 files changed, 30 insertions(+), 93 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index 5829eee99744..b955baccf7bc 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -14,12 +14,10 @@ import CommandInjectionRuntimeExec import ExecUserFlow::PathGraph -class RemoteSource extends Source instanceof RemoteFlowSource { } - from - ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, MethodAccess call, - DataFlow::Node sourceCmd, DataFlow::Node sinkCmd -where callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, + DataFlow::Node sinkCmd +where callIsTaintedByUserInputAndDangerousCommand(source, sink, sourceCmd, sinkCmd) 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 index e815ed5758d4..07476810595c 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -3,108 +3,51 @@ import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources -// a static string of an unsafe executable tainting arg 0 of Runtime.exec() -deprecated 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 - ) - } -} - module ExecCmdFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof StringLiteral and - source.asExpr().(StringLiteral).getValue() instanceof UnSafeExecutable + source.asExpr().(CompileTimeConstantExpr).getStringValue() instanceof UnSafeExecutable } predicate isSink(DataFlow::Node sink) { - exists(RuntimeExecMethod method, MethodAccess call | - call.getMethod() = method and + exists(MethodAccess call | + call.getMethod() instanceof RuntimeExecMethod and sink.asExpr() = call.getArgument(0) and sink.asExpr().getType() instanceof Array ) } predicate isBarrier(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 - ) + node instanceof AssignToNonZeroIndex or + node instanceof ArrayInitAtNonZeroIndex or + node instanceof StreamConcatAtNonZeroIndex or + node.getType() instanceof PrimitiveType or + node.getType() instanceof BoxedType } } /** Tracks flow of unvalidated user input that is used in Runtime.Exec */ module ExecCmdFlow = TaintTracking::Global; -abstract class Source extends DataFlow::Node { - Source() { this = this } -} - -// taint flow from user data to args of the command -deprecated class ExecTaintConfiguration2 extends TaintTracking::Configuration { - ExecTaintConfiguration2() { this = "ExecTaintConfiguration2" } +abstract class Source extends DataFlow::Node { } - override predicate isSource(DataFlow::Node source) { source instanceof Source } - - override predicate isSink(DataFlow::Node sink) { - exists(RuntimeExecMethod method, MethodAccess call | - call.getMethod() = method and - sink.asExpr() = call.getArgument(_) and - sink.asExpr().getType() instanceof Array - ) - } +class RemoteSource extends Source instanceof RemoteFlowSource { } - override predicate isSanitizer(DataFlow::Node node) { - node.asExpr().getFile().isSourceFile() and - ( - node.getType() instanceof PrimitiveType or - node.getType() instanceof BoxedType - ) - } -} +class LocalSource extends Source instanceof LocalUserInput { } module ExecUserFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof Source } predicate isSink(DataFlow::Node sink) { - exists(RuntimeExecMethod method, MethodAccess call | - call.getMethod() = method and + exists(MethodAccess call | + call.getMethod() instanceof RuntimeExecMethod and sink.asExpr() = call.getArgument(_) and sink.asExpr().getType() instanceof Array ) } predicate isBarrier(DataFlow::Node node) { - node.asExpr().getFile().isSourceFile() and - ( - node.getType() instanceof PrimitiveType or - node.getType() instanceof BoxedType - ) + node.getType() instanceof PrimitiveType or + node.getType() instanceof BoxedType } } @@ -116,7 +59,7 @@ class AssignToNonZeroIndex extends DataFlow::Node { AssignToNonZeroIndex() { exists(AssignExpr assign, ArrayAccess access | assign.getDest() = access and - access.getIndexExpr().(IntegerLiteral).getValue() != "0" and + access.getIndexExpr().(IntegerLiteral).getValue().toInt() != 0 and assign.getSource() = this.asExpr() ) } @@ -143,7 +86,7 @@ class StreamConcatAtNonZeroIndex extends DataFlow::Node { } } -// allow list of executables that execute their arguments +// list of executables that execute their arguments // TODO: extend with data extensions class UnSafeExecutable extends string { bindingset[this] @@ -154,17 +97,15 @@ class UnSafeExecutable extends string { } predicate callIsTaintedByUserInputAndDangerousCommand( - MethodAccess call, ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, - DataFlow::Node sourceCmd, DataFlow::Node sinkCmd + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, + DataFlow::Node sinkCmd ) { - call.getMethod() instanceof RuntimeExecMethod and - // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) - ( + exists(MethodAccess call | + call.getMethod() instanceof RuntimeExecMethod and + // this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...}) ExecCmdFlow::flow(sourceCmd, sinkCmd) and - sinkCmd.asExpr() = call.getArgument(0) - ) and - // it is tainted by untrusted user input - ( + sinkCmd.asExpr() = call.getArgument(0) and + // it is tainted by untrusted user input ExecUserFlow::flowPath(source, sink) and sink.getNode().asExpr() = call.getArgument(0) ) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index ba6571530604..524b2acb757f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -15,12 +15,10 @@ import CommandInjectionRuntimeExec import ExecUserFlow::PathGraph -class LocalSource extends Source instanceof LocalUserInput { } - from - ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, MethodAccess call, - DataFlow::Node sourceCmd, DataFlow::Node sinkCmd -where callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd) + ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, + DataFlow::Node sinkCmd +where callIsTaintedByUserInputAndDangerousCommand(source, sink, sourceCmd, sinkCmd) 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() From e9bad321b62be57dc36703d659834513682234b3 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 4 Aug 2023 09:21:45 +0200 Subject: [PATCH 14/15] Apply suggestions from code review --- .../Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp | 5 ----- .../CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp | 5 ----- 2 files changed, 10 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp index c5d7b553a672..820f5b71405f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp @@ -37,10 +37,5 @@ OWASP:
  • 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.qhelp b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp index c8a3beba81c0..be2b7aac973b 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp @@ -37,10 +37,5 @@ OWASP:
  • SEI CERT Oracle Coding Standard for Java: IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method.
    • - - - - From 586c8803c57cbc82c7cf34361894ed73091ad42f Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 4 Aug 2023 10:02:56 +0200 Subject: [PATCH 15/15] Move the sources back the .ql files Otherwise they would both apply at the same time, making both versions of the query identical. --- .../Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql | 2 ++ .../Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll | 4 ---- .../Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql index b955baccf7bc..8c020b6f22b0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql @@ -14,6 +14,8 @@ import CommandInjectionRuntimeExec import ExecUserFlow::PathGraph +class RemoteSource extends Source instanceof RemoteFlowSource { } + from ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll index 07476810595c..7aeef61b58b2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll @@ -30,10 +30,6 @@ module ExecCmdFlow = TaintTracking::Global; abstract class Source extends DataFlow::Node { } -class RemoteSource extends Source instanceof RemoteFlowSource { } - -class LocalSource extends Source instanceof LocalUserInput { } - module ExecUserFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof Source } diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql index 524b2acb757f..037b331609d2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql @@ -15,6 +15,8 @@ import CommandInjectionRuntimeExec import ExecUserFlow::PathGraph +class LocalSource extends Source instanceof LocalUserInput { } + from ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd