From 921b8e80a2d328c34553a082df1008e8729f7144 Mon Sep 17 00:00:00 2001 From: haby0 Date: Wed, 26 May 2021 08:58:29 +0800 Subject: [PATCH 1/9] Jshell Injection --- .../Security/CWE/CWE-094/JShellInjection.java | 26 ++++++++++++++++ .../CWE/CWE-094/JShellInjection.qhelp | 31 +++++++++++++++++++ .../Security/CWE/CWE-094/JShellInjection.ql | 29 +++++++++++++++++ .../Security/CWE/CWE-094/JShellInjection.qll | 28 +++++++++++++++++ .../security/CWE-094/JShellInjection.expected | 11 +++++++ .../security/CWE-094/JShellInjection.java | 26 ++++++++++++++++ .../security/CWE-094/JShellInjection.qlref | 1 + 7 files changed, 152 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll create mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java new file mode 100644 index 00000000000..8d1257697b9 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java @@ -0,0 +1,26 @@ +import javax.servlet.http.HttpServletRequest; +import jdk.jshell.JShell; +import jdk.jshell.SourceCodeAnalysis; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; + +@Controller +public class JShellInjection { + + @GetMapping(value = "bad1") + public void bad1(HttpServletRequest request) { + String input = request.getParameter("code"); + JShell jShell = JShell.builder().build(); + // BAD: allow execution of arbitrary Java code + jShell.eval(input); + } + + @GetMapping(value = "bad2") + public void bad2(HttpServletRequest request) { + String input = request.getParameter("code"); + JShell jShell = JShell.builder().build(); + SourceCodeAnalysis sourceCodeAnalysis = jShell.sourceCodeAnalysis(); + // BAD: allow execution of arbitrary Java code + sourceCodeAnalysis.wrappers(input); + } +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp new file mode 100644 index 00000000000..cae528f5ef1 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp @@ -0,0 +1,31 @@ + + + + +

The Java Shell tool (JShell) is an interactive tool for learning the Java programming +language and prototyping Java code. JShell is a Read-Evaluate-Print Loop (REPL), which +evaluates declarations, statements, and expressions as they are entered and immediately +shows the results. If an expression is built using attacker-controlled data and then evaluated, +it may allow the attacker to run arbitrary code.

+ + + +

It is generally recommended to avoid using untrusted input in a JShell expression. +If it is not possible,JShell expressions should be run in a sandbox that allows accessing only +explicitly allowed classes.

+ + + +

The following example calls JShell.eval(...) or SourceCodeAnalysis.wrappers(...) +to execute untrusted data.

+ + + + +
  • +Java 9 jshell tutorial: JShell introduction +
    • +     +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql new file mode 100644 index 00000000000..2f776500a84 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql @@ -0,0 +1,29 @@ +/** + * @name JShell injection + * @description Evaluation of a user-controlled JShell expression + * may lead to arbitrary code execution. + * @kind path-problem + * @problem.severity error + * @precision high + * @id java/jshell-injection + * @tags security + * external/cwe-094 + */ + +import java +import JShellInjection +import semmle.code.java.dataflow.FlowSources +import DataFlow::PathGraph + +class JShellInjectionConfiguration extends TaintTracking::Configuration { + JShellInjectionConfiguration() { this = "JShellInjectionConfiguration" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, JShellInjectionConfiguration conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "JShell injection from $@.", source.getNode(), + "this user input" diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll new file mode 100644 index 00000000000..20c394f6bd8 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll @@ -0,0 +1,28 @@ +import java +import semmle.code.java.dataflow.FlowSources + +/** A sink for JShell expression injection vulnerabilities. */ +class JShellInjectionSink extends DataFlow::Node { + JShellInjectionSink() { + this.asExpr() = any(JShellEvalCall jsec).getArgument(0) or + this.asExpr() = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) + } +} + +/** A call to `JShell.eval`. */ +class JShellEvalCall extends MethodAccess { + JShellEvalCall() { + this.getMethod().hasName("eval") and + this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "JShell") and + this.getMethod().getNumberOfParameters() = 1 + } +} + +/** A call to `SourceCodeAnalysis.wrappers`. */ +class SourceCodeAnalysisWrappersCall extends MethodAccess { + SourceCodeAnalysisWrappersCall() { + this.getMethod().hasName("wrappers") and + this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and + this.getMethod().getNumberOfParameters() = 1 + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected new file mode 100644 index 00000000000..6c7f32fc94d --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected @@ -0,0 +1,11 @@ +edges +| JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input | +| JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input | +nodes +| JShellInjection.java:12:18:12:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| JShellInjection.java:15:15:15:19 | input | semmle.label | input | +| JShellInjection.java:20:18:20:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| JShellInjection.java:24:31:24:35 | input | semmle.label | input | +#select +| JShellInjection.java:15:15:15:19 | input | JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input | JShell injection from $@. | JShellInjection.java:12:18:12:45 | getParameter(...) | this user input | +| JShellInjection.java:24:31:24:35 | input | JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input | JShell injection from $@. | JShellInjection.java:20:18:20:45 | getParameter(...) | this user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java new file mode 100644 index 00000000000..8d1257697b9 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java @@ -0,0 +1,26 @@ +import javax.servlet.http.HttpServletRequest; +import jdk.jshell.JShell; +import jdk.jshell.SourceCodeAnalysis; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; + +@Controller +public class JShellInjection { + + @GetMapping(value = "bad1") + public void bad1(HttpServletRequest request) { + String input = request.getParameter("code"); + JShell jShell = JShell.builder().build(); + // BAD: allow execution of arbitrary Java code + jShell.eval(input); + } + + @GetMapping(value = "bad2") + public void bad2(HttpServletRequest request) { + String input = request.getParameter("code"); + JShell jShell = JShell.builder().build(); + SourceCodeAnalysis sourceCodeAnalysis = jShell.sourceCodeAnalysis(); + // BAD: allow execution of arbitrary Java code + sourceCodeAnalysis.wrappers(input); + } +} \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.qlref b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.qlref new file mode 100644 index 00000000000..d2128dd2058 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-094/JShellInjection.ql \ No newline at end of file From ed0aabef46b4b3f44122d6499e64342f4da5030d Mon Sep 17 00:00:00 2001 From: haby0 Date: Wed, 26 May 2021 17:48:10 +0800 Subject: [PATCH 2/9] add isAdditionalTaintStep --- .../Security/CWE/CWE-094/JShellInjection.java | 18 ++++++++++++++++-- .../Security/CWE/CWE-094/JShellInjection.ql | 13 +++++++++++++ .../Security/CWE/CWE-094/JShellInjection.qll | 18 +++++++++++++++++- .../security/CWE-094/JShellInjection.expected | 4 ++++ .../security/CWE-094/JShellInjection.java | 18 ++++++++++++++++-- 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java index 8d1257697b9..115030087ff 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java @@ -11,7 +11,7 @@ public class JShellInjection { public void bad1(HttpServletRequest request) { String input = request.getParameter("code"); JShell jShell = JShell.builder().build(); - // BAD: allow execution of arbitrary Java code + // BAD: allow execution of arbitrary Java code jShell.eval(input); } @@ -20,7 +20,21 @@ public class JShellInjection { String input = request.getParameter("code"); JShell jShell = JShell.builder().build(); SourceCodeAnalysis sourceCodeAnalysis = jShell.sourceCodeAnalysis(); - // BAD: allow execution of arbitrary Java code + // BAD: allow execution of arbitrary Java code sourceCodeAnalysis.wrappers(input); } + + @GetMapping(value = "bad3") + public void bad3(HttpServletRequest request) { + String input = request.getParameter("code"); + JShell jShell = JShell.builder().build(); + SourceCodeAnalysis.CompletionInfo info; + SourceCodeAnalysis sca = jShell.sourceCodeAnalysis(); + for (info = sca.analyzeCompletion(input); + info.completeness().isComplete(); + info = sca.analyzeCompletion(info.remaining())) { + // BAD: allow execution of arbitrary Java code + jShell.eval(info.source()); + } + } } \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql index 2f776500a84..8fc84ab0d97 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql @@ -21,6 +21,19 @@ class JShellInjectionConfiguration extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } + + override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) { + exists(MethodAccess ma | + ma.getMethod().hasName("analyzeCompletion") and + ma.getMethod().getNumberOfParameters() = 1 and + ma.getMethod() + .getDeclaringType() + .getASupertype*() + .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and + ma.getArgument(0) = prod.asExpr() and + ma = succ.asExpr() + ) + } } from DataFlow::PathNode source, DataFlow::PathNode sink, JShellInjectionConfiguration conf diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll index 20c394f6bd8..410e5acffa8 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll @@ -4,8 +4,24 @@ import semmle.code.java.dataflow.FlowSources /** A sink for JShell expression injection vulnerabilities. */ class JShellInjectionSink extends DataFlow::Node { JShellInjectionSink() { - this.asExpr() = any(JShellEvalCall jsec).getArgument(0) or + this.asExpr() = any(JShellEvalCall jsec).getArgument(0) + or this.asExpr() = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) + or + exists(MethodAccess ma | + ma.getMethod().hasName("source") and + ma.getMethod().getNumberOfParameters() = 0 and + ma.getMethod() + .getDeclaringType() + .getASupertype*() + .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis$CompletionInfo") and + ma.getQualifier() = this.asExpr() and + ( + ma = any(JShellEvalCall jsec).getArgument(0) + or + ma = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) + ) + ) } } diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected index 6c7f32fc94d..a6b227de39e 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected @@ -1,11 +1,15 @@ edges | JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input | | JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input | +| JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:19 | info | nodes | JShellInjection.java:12:18:12:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | | JShellInjection.java:15:15:15:19 | input | semmle.label | input | | JShellInjection.java:20:18:20:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | | JShellInjection.java:24:31:24:35 | input | semmle.label | input | +| JShellInjection.java:29:18:29:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| JShellInjection.java:37:16:37:19 | info | semmle.label | info | #select | JShellInjection.java:15:15:15:19 | input | JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input | JShell injection from $@. | JShellInjection.java:12:18:12:45 | getParameter(...) | this user input | | JShellInjection.java:24:31:24:35 | input | JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input | JShell injection from $@. | JShellInjection.java:20:18:20:45 | getParameter(...) | this user input | +| JShellInjection.java:37:16:37:19 | info | JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:19 | info | JShell injection from $@. | JShellInjection.java:29:18:29:45 | getParameter(...) | this user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java index 8d1257697b9..115030087ff 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java @@ -11,7 +11,7 @@ public class JShellInjection { public void bad1(HttpServletRequest request) { String input = request.getParameter("code"); JShell jShell = JShell.builder().build(); - // BAD: allow execution of arbitrary Java code + // BAD: allow execution of arbitrary Java code jShell.eval(input); } @@ -20,7 +20,21 @@ public class JShellInjection { String input = request.getParameter("code"); JShell jShell = JShell.builder().build(); SourceCodeAnalysis sourceCodeAnalysis = jShell.sourceCodeAnalysis(); - // BAD: allow execution of arbitrary Java code + // BAD: allow execution of arbitrary Java code sourceCodeAnalysis.wrappers(input); } + + @GetMapping(value = "bad3") + public void bad3(HttpServletRequest request) { + String input = request.getParameter("code"); + JShell jShell = JShell.builder().build(); + SourceCodeAnalysis.CompletionInfo info; + SourceCodeAnalysis sca = jShell.sourceCodeAnalysis(); + for (info = sca.analyzeCompletion(input); + info.completeness().isComplete(); + info = sca.analyzeCompletion(info.remaining())) { + // BAD: allow execution of arbitrary Java code + jShell.eval(info.source()); + } + } } \ No newline at end of file From 3a2a99e28957d76a4f1969fce50afec3452b6d29 Mon Sep 17 00:00:00 2001 From: haby0 Date: Wed, 2 Jun 2021 09:18:22 +0800 Subject: [PATCH 3/9] Fix 1 --- .../experimental/Security/CWE/CWE-094/JShellInjection.qhelp | 4 ++-- .../src/experimental/Security/CWE/CWE-094/JShellInjection.ql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp index cae528f5ef1..44a95f9d399 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp @@ -13,7 +13,7 @@ it may allow the attacker to run arbitrary code.

    It is generally recommended to avoid using untrusted input in a JShell expression. -If it is not possible,JShell expressions should be run in a sandbox that allows accessing only +If it is not possible, JShell expressions should be run in a sandbox that allows accessing only explicitly allowed classes.

     @@ -25,7 +25,7 @@ to execute untrusted data.

  • -Java 9 jshell tutorial: JShell introduction +Introduction to JShell: Java Shell User’s Guide
    • diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql index 8fc84ab0d97..2754d58be54 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql @@ -22,7 +22,7 @@ class JShellInjectionConfiguration extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } - override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) { + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { exists(MethodAccess ma | ma.getMethod().hasName("analyzeCompletion") and ma.getMethod().getNumberOfParameters() = 1 and @@ -30,7 +30,7 @@ class JShellInjectionConfiguration extends TaintTracking::Configuration { .getDeclaringType() .getASupertype*() .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and - ma.getArgument(0) = prod.asExpr() and + ma.getArgument(0) = pred.asExpr() and ma = succ.asExpr() ) } From bfe0d40987954c704f07aa7ff4c4789e538cdd2b Mon Sep 17 00:00:00 2001 From: haby0 Date: Wed, 16 Jun 2021 13:50:53 +0800 Subject: [PATCH 4/9] using isAdditionalTaintStep --- .../Security/CWE/CWE-094/JShellInjection.ql | 16 +++---- .../Security/CWE/CWE-094/JShellInjection.qll | 43 +++++++++++-------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql index 2754d58be54..9b50d500c58 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql @@ -12,6 +12,7 @@ import java import JShellInjection +import semmle.code.java.dataflow.DataFlow2 import semmle.code.java.dataflow.FlowSources import DataFlow::PathGraph @@ -23,15 +24,12 @@ class JShellInjectionConfiguration extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(MethodAccess ma | - ma.getMethod().hasName("analyzeCompletion") and - ma.getMethod().getNumberOfParameters() = 1 and - ma.getMethod() - .getDeclaringType() - .getASupertype*() - .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and - ma.getArgument(0) = pred.asExpr() and - ma = succ.asExpr() + exists( + SourceCodeAnalysisAnalyzeCompletionCall scaacc, CompletionInfoSourceOrRemainingCall cisorc + | + scaacc.getArgument(0) = pred.asExpr() and + cisorc = succ.asExpr() and + DataFlow2::localExprFlow(scaacc, cisorc.getQualifier()) ) } } diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll index 410e5acffa8..894cd03ce67 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll @@ -7,26 +7,11 @@ class JShellInjectionSink extends DataFlow::Node { this.asExpr() = any(JShellEvalCall jsec).getArgument(0) or this.asExpr() = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) - or - exists(MethodAccess ma | - ma.getMethod().hasName("source") and - ma.getMethod().getNumberOfParameters() = 0 and - ma.getMethod() - .getDeclaringType() - .getASupertype*() - .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis$CompletionInfo") and - ma.getQualifier() = this.asExpr() and - ( - ma = any(JShellEvalCall jsec).getArgument(0) - or - ma = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) - ) - ) } } /** A call to `JShell.eval`. */ -class JShellEvalCall extends MethodAccess { +private class JShellEvalCall extends MethodAccess { JShellEvalCall() { this.getMethod().hasName("eval") and this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "JShell") and @@ -35,10 +20,34 @@ class JShellEvalCall extends MethodAccess { } /** A call to `SourceCodeAnalysis.wrappers`. */ -class SourceCodeAnalysisWrappersCall extends MethodAccess { +private class SourceCodeAnalysisWrappersCall extends MethodAccess { SourceCodeAnalysisWrappersCall() { this.getMethod().hasName("wrappers") and this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and this.getMethod().getNumberOfParameters() = 1 } } + +/** A call to `SourceCodeAnalysis.analyzeCompletion`. */ +class SourceCodeAnalysisAnalyzeCompletionCall extends MethodAccess { + SourceCodeAnalysisAnalyzeCompletionCall() { + this.getMethod().hasName("analyzeCompletion") and + this.getMethod() + .getDeclaringType() + .getASupertype*() + .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and + this.getMethod().getNumberOfParameters() = 1 + } +} + +/** A call to `CompletionInfo.source` or `CompletionInfo.remaining`. */ +class CompletionInfoSourceOrRemainingCall extends MethodAccess { + CompletionInfoSourceOrRemainingCall() { + this.getMethod().getName() in ["source", "remaining"] and + this.getMethod() + .getDeclaringType() + .getASupertype*() + .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis$CompletionInfo") and + this.getMethod().getNumberOfParameters() = 0 + } +} From a71757f0f4fbf43c3d0c36d54ebe13cd54e54e1d Mon Sep 17 00:00:00 2001 From: haby0 Date: Thu, 3 Jun 2021 16:04:29 +0800 Subject: [PATCH 5/9] Update java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp Co-authored-by: Marcono1234 --- .../src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp index 44a95f9d399..05457c8fd82 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp @@ -25,7 +25,7 @@ to execute untrusted data.

  • -Introduction to JShell: Java Shell User’s Guide +Java Shell User’s Guide: Introduction to JShell
    • From 2b77f7d1bc73d7616d507c9348a47a017f667959 Mon Sep 17 00:00:00 2001 From: haby0 Date: Thu, 17 Jun 2021 09:12:06 +0800 Subject: [PATCH 6/9] Modify isAdditionalTaintStep --- .../Security/CWE/CWE-094/JShellInjection.ql | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql index 9b50d500c58..62dbdcba670 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql @@ -12,7 +12,6 @@ import java import JShellInjection -import semmle.code.java.dataflow.DataFlow2 import semmle.code.java.dataflow.FlowSources import DataFlow::PathGraph @@ -24,12 +23,12 @@ class JShellInjectionConfiguration extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - exists( - SourceCodeAnalysisAnalyzeCompletionCall scaacc, CompletionInfoSourceOrRemainingCall cisorc - | - scaacc.getArgument(0) = pred.asExpr() and - cisorc = succ.asExpr() and - DataFlow2::localExprFlow(scaacc, cisorc.getQualifier()) + exists(SourceCodeAnalysisAnalyzeCompletionCall scaacc | + scaacc.getArgument(0) = pred.asExpr() and scaacc = succ.asExpr() + ) + or + exists(CompletionInfoSourceOrRemainingCall cisorc | + cisorc.getQualifier() = pred.asExpr() and cisorc = succ.asExpr() ) } } From dca737190b8914846a533d35e7c3d0d4387eb650 Mon Sep 17 00:00:00 2001 From: haby0 Date: Thu, 17 Jun 2021 17:20:07 +0800 Subject: [PATCH 7/9] Modify JShellInjection.expected --- .../query-tests/security/CWE-094/JShellInjection.expected | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected index a6b227de39e..43f86caaf6d 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected @@ -1,15 +1,15 @@ edges | JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input | | JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input | -| JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:19 | info | +| JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:28 | source(...) | nodes | JShellInjection.java:12:18:12:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | | JShellInjection.java:15:15:15:19 | input | semmle.label | input | | JShellInjection.java:20:18:20:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | | JShellInjection.java:24:31:24:35 | input | semmle.label | input | | JShellInjection.java:29:18:29:45 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| JShellInjection.java:37:16:37:19 | info | semmle.label | info | +| JShellInjection.java:37:16:37:28 | source(...) | semmle.label | source(...) | #select | JShellInjection.java:15:15:15:19 | input | JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input | JShell injection from $@. | JShellInjection.java:12:18:12:45 | getParameter(...) | this user input | | JShellInjection.java:24:31:24:35 | input | JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input | JShell injection from $@. | JShellInjection.java:20:18:20:45 | getParameter(...) | this user input | -| JShellInjection.java:37:16:37:19 | info | JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:19 | info | JShell injection from $@. | JShellInjection.java:29:18:29:45 | getParameter(...) | this user input | +| JShellInjection.java:37:16:37:28 | source(...) | JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:28 | source(...) | JShell injection from $@. | JShellInjection.java:29:18:29:45 | getParameter(...) | this user input | From 1750efad2a88de88b32789a8a4430e8c352b59f1 Mon Sep 17 00:00:00 2001 From: haby0 Date: Fri, 18 Jun 2021 21:46:48 +0800 Subject: [PATCH 8/9] fix --- .../query-tests/security/CWE-094/options | 2 +- .../test/stubs/jshell/jdk/jshell/JShell.java | 37 ++++++ .../test/stubs/jshell/jdk/jshell/Snippet.java | 31 +++++ .../stubs/jshell/jdk/jshell/SnippetEvent.java | 5 + .../jshell/jdk/jshell/SourceCodeAnalysis.java | 111 ++++++++++++++++++ 5 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 java/ql/test/stubs/jshell/jdk/jshell/JShell.java create mode 100644 java/ql/test/stubs/jshell/jdk/jshell/Snippet.java create mode 100644 java/ql/test/stubs/jshell/jdk/jshell/SnippetEvent.java create mode 100644 java/ql/test/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/options b/java/ql/test/experimental/query-tests/security/CWE-094/options index 1fc637be50f..22891626ba4 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/options +++ b/java/ql/test/experimental/query-tests/security/CWE-094/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5 \ No newline at end of file +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../stubs/jshell \ No newline at end of file diff --git a/java/ql/test/stubs/jshell/jdk/jshell/JShell.java b/java/ql/test/stubs/jshell/jdk/jshell/JShell.java new file mode 100644 index 00000000000..fe49d13cd6e --- /dev/null +++ b/java/ql/test/stubs/jshell/jdk/jshell/JShell.java @@ -0,0 +1,37 @@ +package jdk.jshell; + +import java.util.List; +import java.lang.IllegalStateException; + +public class JShell implements AutoCloseable { + + JShell(Builder b) throws IllegalStateException { } + + public static class Builder { + + Builder() { } + + public JShell build() throws IllegalStateException { + return null; + } + } + + public static JShell create() throws IllegalStateException { + return null; + } + + public static Builder builder() { + return null; + } + + public SourceCodeAnalysis sourceCodeAnalysis() { + return null; + } + + public List eval(String input) throws IllegalStateException { + return null; + } + + @Override + public void close() { } +} diff --git a/java/ql/test/stubs/jshell/jdk/jshell/Snippet.java b/java/ql/test/stubs/jshell/jdk/jshell/Snippet.java new file mode 100644 index 00000000000..38da9b6cc35 --- /dev/null +++ b/java/ql/test/stubs/jshell/jdk/jshell/Snippet.java @@ -0,0 +1,31 @@ +package jdk.jshell; + +public abstract class Snippet { + + public enum Kind { + + IMPORT(true), + + TYPE_DECL(true), + + METHOD(true), + + VAR(true), + + EXPRESSION(false), + + STATEMENT(false), + + ERRONEOUS(false); + + private final boolean isPersistent; + + Kind(boolean isPersistent) { + this.isPersistent = isPersistent; + } + + public boolean isPersistent() { + return false; + } + } +} diff --git a/java/ql/test/stubs/jshell/jdk/jshell/SnippetEvent.java b/java/ql/test/stubs/jshell/jdk/jshell/SnippetEvent.java new file mode 100644 index 00000000000..9425a278a6c --- /dev/null +++ b/java/ql/test/stubs/jshell/jdk/jshell/SnippetEvent.java @@ -0,0 +1,5 @@ +package jdk.jshell; + +public class SnippetEvent { + +} diff --git a/java/ql/test/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java b/java/ql/test/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java new file mode 100644 index 00000000000..7f629a46cbd --- /dev/null +++ b/java/ql/test/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java @@ -0,0 +1,111 @@ +package jdk.jshell; + +import java.util.Collection; +import java.util.List; + +public abstract class SourceCodeAnalysis { + + public abstract CompletionInfo analyzeCompletion(String input); + + public abstract List completionSuggestions(String input, int cursor, int[] anchor); + + public abstract List documentation(String input, int cursor, boolean computeJavadoc); + + public abstract String analyzeType(String code, int cursor); + + public abstract QualifiedNames listQualifiedNames(String code, int cursor); + + public abstract SnippetWrapper wrapper(Snippet snippet); + + public abstract List wrappers(String input); + + public abstract Collection dependents(Snippet snippet); + + SourceCodeAnalysis() {} + + public interface CompletionInfo { + + Completeness completeness(); + + String remaining(); + + String source(); + } + + public enum Completeness { + + COMPLETE(true), + + COMPLETE_WITH_SEMI(true), + + DEFINITELY_INCOMPLETE(false), + + CONSIDERED_INCOMPLETE(false), + + EMPTY(false), + + UNKNOWN(true); + + private final boolean isComplete; + + Completeness(boolean isComplete) { + this.isComplete = isComplete; + } + + public boolean isComplete() { + return isComplete; + } + } + + public interface Suggestion { + + String continuation(); + + boolean matchesType(); + } + + public interface Documentation { + + String signature(); + + String javadoc(); + } + + public static final class QualifiedNames { + + + QualifiedNames(List names, int simpleNameLength, boolean upToDate, boolean resolvable) { } + + public List getNames() { + return null; + } + + public int getSimpleNameLength() { + return 1; + } + + public boolean isUpToDate() { + return false; + } + + public boolean isResolvable() { + return false; + } + + } + + public interface SnippetWrapper { + + String source(); + + String wrapped(); + + String fullClassName(); + + Snippet.Kind kind(); + + int sourceToWrappedPosition(int pos); + + int wrappedToSourcePosition(int pos); + } +} From 3cf71c50b84b29efd0053e24c73aa3f70675d02a Mon Sep 17 00:00:00 2001 From: haby0 Date: Thu, 24 Jun 2021 19:24:38 +0800 Subject: [PATCH 9/9] Mobile stubs --- java/ql/test/experimental/query-tests/security/CWE-094/options | 2 +- .../test/{ => experimental}/stubs/jshell/jdk/jshell/JShell.java | 0 .../{ => experimental}/stubs/jshell/jdk/jshell/Snippet.java | 0 .../stubs/jshell/jdk/jshell/SnippetEvent.java | 0 .../stubs/jshell/jdk/jshell/SourceCodeAnalysis.java | 0 5 files changed, 1 insertion(+), 1 deletion(-) rename java/ql/test/{ => experimental}/stubs/jshell/jdk/jshell/JShell.java (100%) rename java/ql/test/{ => experimental}/stubs/jshell/jdk/jshell/Snippet.java (100%) rename java/ql/test/{ => experimental}/stubs/jshell/jdk/jshell/SnippetEvent.java (100%) rename java/ql/test/{ => experimental}/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java (100%) diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/options b/java/ql/test/experimental/query-tests/security/CWE-094/options index 22891626ba4..d54ac82b606 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/options +++ b/java/ql/test/experimental/query-tests/security/CWE-094/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../stubs/jshell \ No newline at end of file +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../experimental/stubs/jshell \ No newline at end of file diff --git a/java/ql/test/stubs/jshell/jdk/jshell/JShell.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/JShell.java similarity index 100% rename from java/ql/test/stubs/jshell/jdk/jshell/JShell.java rename to java/ql/test/experimental/stubs/jshell/jdk/jshell/JShell.java diff --git a/java/ql/test/stubs/jshell/jdk/jshell/Snippet.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/Snippet.java similarity index 100% rename from java/ql/test/stubs/jshell/jdk/jshell/Snippet.java rename to java/ql/test/experimental/stubs/jshell/jdk/jshell/Snippet.java diff --git a/java/ql/test/stubs/jshell/jdk/jshell/SnippetEvent.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/SnippetEvent.java similarity index 100% rename from java/ql/test/stubs/jshell/jdk/jshell/SnippetEvent.java rename to java/ql/test/experimental/stubs/jshell/jdk/jshell/SnippetEvent.java diff --git a/java/ql/test/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java similarity index 100% rename from java/ql/test/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java rename to java/ql/test/experimental/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java