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