Merge pull request #5955 from haby0/java/JShellCodeInjection

Java: JShell Injection

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());
+		}
+	}
+}

java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>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.</p>
+</overview>
+
+<recommendation>
+<p>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.</p>
+</recommendation>
+
+<example>
+<p>The following example calls <code>JShell.eval(...)</code> or <code>SourceCodeAnalysis.wrappers(...)</code> 
+to execute untrusted data.</p>
+<sample src="JShellInjection.java" />
+</example>
+
+<references>
+<li>
+Java Shell User’s Guide: <a href="https://docs.oracle.com/en/java/javase/11/jshell/introduction-jshell.html">Introduction to JShell</a>
+</li>
+</references>
+</qhelp>

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"

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
+  }
+}

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 |

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());
+		}
+	}
+}

java/ql/test/experimental/query-tests/security/CWE-094/JShellInjection.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-094/JShellInjection.ql

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
+//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

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<SnippetEvent> eval(String input) throws IllegalStateException {
+        return null;
+    }
+
+    @Override
+    public void close() { }
+}

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;
+        }
+	}
+}

java/ql/test/experimental/stubs/jshell/jdk/jshell/SnippetEvent.java

@@ -0,0 +1,5 @@
+package jdk.jshell;
+
+public class SnippetEvent {
+
+}

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<Suggestion> completionSuggestions(String input, int cursor, int[] anchor);
+
+    public abstract List<Documentation> 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<SnippetWrapper> wrappers(String input);
+
+    public abstract Collection<Snippet> 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<String> names, int simpleNameLength, boolean upToDate, boolean resolvable) { }
+
+        public List<String> 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);
+    }
+}