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..115030087ff --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.java @@ -0,0 +1,40 @@ +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); + } + + @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.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp new file mode 100644 index 00000000000..05457c8fd82 --- /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 Shell User’s Guide: Introduction to JShell +
    • +     +     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..62dbdcba670 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.ql @@ -0,0 +1,39 @@ +/** + * @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 } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + 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 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..894cd03ce67 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll @@ -0,0 +1,53 @@ +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`. */ +private 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`. */ +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 + } +} 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..43f86caaf6d --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.expected @@ -0,0 +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: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: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: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 | 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..115030087ff --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.java @@ -0,0 +1,40 @@ +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); + } + + @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/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 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..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 \ 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/experimental/stubs/jshell/jdk/jshell/JShell.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/JShell.java new file mode 100644 index 00000000000..fe49d13cd6e --- /dev/null +++ b/java/ql/test/experimental/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/experimental/stubs/jshell/jdk/jshell/Snippet.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/Snippet.java new file mode 100644 index 00000000000..38da9b6cc35 --- /dev/null +++ b/java/ql/test/experimental/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/experimental/stubs/jshell/jdk/jshell/SnippetEvent.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/SnippetEvent.java new file mode 100644 index 00000000000..9425a278a6c --- /dev/null +++ b/java/ql/test/experimental/stubs/jshell/jdk/jshell/SnippetEvent.java @@ -0,0 +1,5 @@ +package jdk.jshell; + +public class SnippetEvent { + +} diff --git a/java/ql/test/experimental/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java b/java/ql/test/experimental/stubs/jshell/jdk/jshell/SourceCodeAnalysis.java new file mode 100644 index 00000000000..7f629a46cbd --- /dev/null +++ b/java/ql/test/experimental/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); + } +}