Skip to content

Commit 2acb4de

Browse files
smowtonsmowton
authored
Merge pull request github#5955 from haby0/java/JShellCodeInjection
Java: JShell Injection
2 parents 95ad8b5 + 3cf71c5 commit 2acb4de

File tree

12 files changed

+404
-1
lines changed

12 files changed

+404
-1
lines changed

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import javax.servlet.http.HttpServletRequest;
2+
import jdk.jshell.JShell;
3+
import jdk.jshell.SourceCodeAnalysis;
4+
import org.springframework.stereotype.Controller;
5+
import org.springframework.web.bind.annotation.GetMapping;
6+
7+
@Controller
8+
public class JShellInjection {
9+
10+
@GetMapping(value = "bad1")
11+
public void bad1(HttpServletRequest request) {
12+
String input = request.getParameter("code");
13+
JShell jShell = JShell.builder().build();
14+
// BAD: allow execution of arbitrary Java code
15+
jShell.eval(input);
16+
}
17+
18+
@GetMapping(value = "bad2")
19+
public void bad2(HttpServletRequest request) {
20+
String input = request.getParameter("code");
21+
JShell jShell = JShell.builder().build();
22+
SourceCodeAnalysis sourceCodeAnalysis = jShell.sourceCodeAnalysis();
23+
// BAD: allow execution of arbitrary Java code
24+
sourceCodeAnalysis.wrappers(input);
25+
}
26+
27+
@GetMapping(value = "bad3")
28+
public void bad3(HttpServletRequest request) {
29+
String input = request.getParameter("code");
30+
JShell jShell = JShell.builder().build();
31+
SourceCodeAnalysis.CompletionInfo info;
32+
SourceCodeAnalysis sca = jShell.sourceCodeAnalysis();
33+
for (info = sca.analyzeCompletion(input);
34+
info.completeness().isComplete();
35+
info = sca.analyzeCompletion(info.remaining())) {
36+
// BAD: allow execution of arbitrary Java code
37+
jShell.eval(info.source());
38+
}
39+
}
40+
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>The Java Shell tool (JShell) is an interactive tool for learning the Java programming
8+
language and prototyping Java code. JShell is a Read-Evaluate-Print Loop (REPL), which
9+
evaluates declarations, statements, and expressions as they are entered and immediately
10+
shows the results. If an expression is built using attacker-controlled data and then evaluated,
11+
it may allow the attacker to run arbitrary code.</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>It is generally recommended to avoid using untrusted input in a JShell expression.
16+
If it is not possible, JShell expressions should be run in a sandbox that allows accessing only
17+
explicitly allowed classes.</p>
18+
</recommendation>
19+
20+
<example>
21+
<p>The following example calls <code>JShell.eval(...)</code> or <code>SourceCodeAnalysis.wrappers(...)</code>
22+
to execute untrusted data.</p>
23+
<sample src="JShellInjection.java" />
24+
</example>
25+
26+
<references>
27+
<li>
28+
Java Shell User’s Guide: <a href="https://docs.oracle.com/en/java/javase/11/jshell/introduction-jshell.html">Introduction to JShell</a>
29+
</li>
30+
</references>
31+
</qhelp>

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* @name JShell injection
3+
* @description Evaluation of a user-controlled JShell expression
4+
* may lead to arbitrary code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/jshell-injection
9+
* @tags security
10+
* external/cwe-094
11+
*/
12+
13+
import java
14+
import JShellInjection
15+
import semmle.code.java.dataflow.FlowSources
16+
import DataFlow::PathGraph
17+
18+
class JShellInjectionConfiguration extends TaintTracking::Configuration {
19+
JShellInjectionConfiguration() { this = "JShellInjectionConfiguration" }
20+
21+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
22+
23+
override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink }
24+
25+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
26+
exists(SourceCodeAnalysisAnalyzeCompletionCall scaacc |
27+
scaacc.getArgument(0) = pred.asExpr() and scaacc = succ.asExpr()
28+
)
29+
or
30+
exists(CompletionInfoSourceOrRemainingCall cisorc |
31+
cisorc.getQualifier() = pred.asExpr() and cisorc = succ.asExpr()
32+
)
33+
}
34+
}
35+
36+
from DataFlow::PathNode source, DataFlow::PathNode sink, JShellInjectionConfiguration conf
37+
where conf.hasFlowPath(source, sink)
38+
select sink.getNode(), source, sink, "JShell injection from $@.", source.getNode(),
39+
"this user input"

‎java/ql/src/experimental/Security/CWE/CWE-094/JShellInjection.qll

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
4+
/** A sink for JShell expression injection vulnerabilities. */
5+
class JShellInjectionSink extends DataFlow::Node {
6+
JShellInjectionSink() {
7+
this.asExpr() = any(JShellEvalCall jsec).getArgument(0)
8+
or
9+
this.asExpr() = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0)
10+
}
11+
}
12+
13+
/** A call to `JShell.eval`. */
14+
private class JShellEvalCall extends MethodAccess {
15+
JShellEvalCall() {
16+
this.getMethod().hasName("eval") and
17+
this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "JShell") and
18+
this.getMethod().getNumberOfParameters() = 1
19+
}
20+
}
21+
22+
/** A call to `SourceCodeAnalysis.wrappers`. */
23+
private class SourceCodeAnalysisWrappersCall extends MethodAccess {
24+
SourceCodeAnalysisWrappersCall() {
25+
this.getMethod().hasName("wrappers") and
26+
this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and
27+
this.getMethod().getNumberOfParameters() = 1
28+
}
29+
}
30+
31+
/** A call to `SourceCodeAnalysis.analyzeCompletion`. */
32+
class SourceCodeAnalysisAnalyzeCompletionCall extends MethodAccess {
33+
SourceCodeAnalysisAnalyzeCompletionCall() {
34+
this.getMethod().hasName("analyzeCompletion") and
35+
this.getMethod()
36+
.getDeclaringType()
37+
.getASupertype*()
38+
.hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and
39+
this.getMethod().getNumberOfParameters() = 1
40+
}
41+
}
42+
43+
/** A call to `CompletionInfo.source` or `CompletionInfo.remaining`. */
44+
class CompletionInfoSourceOrRemainingCall extends MethodAccess {
45+
CompletionInfoSourceOrRemainingCall() {
46+
this.getMethod().getName() in ["source", "remaining"] and
47+
this.getMethod()
48+
.getDeclaringType()
49+
.getASupertype*()
50+
.hasQualifiedName("jdk.jshell", "SourceCodeAnalysis$CompletionInfo") and
51+
this.getMethod().getNumberOfParameters() = 0
52+
}
53+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
edges
2+
| JShellInjection.java:12:18:12:45 | getParameter(...) : String | JShellInjection.java:15:15:15:19 | input |
3+
| JShellInjection.java:20:18:20:45 | getParameter(...) : String | JShellInjection.java:24:31:24:35 | input |
4+
| JShellInjection.java:29:18:29:45 | getParameter(...) : String | JShellInjection.java:37:16:37:28 | source(...) |
5+
nodes
6+
| JShellInjection.java:12:18:12:45 | getParameter(...) : String | semmle.label | getParameter(...) : String |
7+
| JShellInjection.java:15:15:15:19 | input | semmle.label | input |
8+
| JShellInjection.java:20:18:20:45 | getParameter(...) : String | semmle.label | getParameter(...) : String |
9+
| JShellInjection.java:24:31:24:35 | input | semmle.label | input |
10+
| JShellInjection.java:29:18:29:45 | getParameter(...) : String | semmle.label | getParameter(...) : String |
11+
| JShellInjection.java:37:16:37:28 | source(...) | semmle.label | source(...) |
12+
#select
13+
| 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 |
14+
| 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 |
15+
| 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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import javax.servlet.http.HttpServletRequest;
2+
import jdk.jshell.JShell;
3+
import jdk.jshell.SourceCodeAnalysis;
4+
import org.springframework.stereotype.Controller;
5+
import org.springframework.web.bind.annotation.GetMapping;
6+
7+
@Controller
8+
public class JShellInjection {
9+
10+
@GetMapping(value = "bad1")
11+
public void bad1(HttpServletRequest request) {
12+
String input = request.getParameter("code");
13+
JShell jShell = JShell.builder().build();
14+
// BAD: allow execution of arbitrary Java code
15+
jShell.eval(input);
16+
}
17+
18+
@GetMapping(value = "bad2")
19+
public void bad2(HttpServletRequest request) {
20+
String input = request.getParameter("code");
21+
JShell jShell = JShell.builder().build();
22+
SourceCodeAnalysis sourceCodeAnalysis = jShell.sourceCodeAnalysis();
23+
// BAD: allow execution of arbitrary Java code
24+
sourceCodeAnalysis.wrappers(input);
25+
}
26+
27+
@GetMapping(value = "bad3")
28+
public void bad3(HttpServletRequest request) {
29+
String input = request.getParameter("code");
30+
JShell jShell = JShell.builder().build();
31+
SourceCodeAnalysis.CompletionInfo info;
32+
SourceCodeAnalysis sca = jShell.sourceCodeAnalysis();
33+
for (info = sca.analyzeCompletion(input);
34+
info.completeness().isComplete();
35+
info = sca.analyzeCompletion(info.remaining())) {
36+
// BAD: allow execution of arbitrary Java code
37+
jShell.eval(info.source());
38+
}
39+
}
40+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-094/JShellInjection.ql

‎java/ql/test/experimental/query-tests/security/CWE-094/options

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +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
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}/../../../../experimental/stubs/jshell

‎java/ql/test/experimental/stubs/jshell/jdk/jshell/JShell.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package jdk.jshell;
2+
3+
import java.util.List;
4+
import java.lang.IllegalStateException;
5+
6+
public class JShell implements AutoCloseable {
7+
8+
JShell(Builder b) throws IllegalStateException { }
9+
10+
public static class Builder {
11+
12+
Builder() { }
13+
14+
public JShell build() throws IllegalStateException {
15+
return null;
16+
}
17+
}
18+
19+
public static JShell create() throws IllegalStateException {
20+
return null;
21+
}
22+
23+
public static Builder builder() {
24+
return null;
25+
}
26+
27+
public SourceCodeAnalysis sourceCodeAnalysis() {
28+
return null;
29+
}
30+
31+
public List<SnippetEvent> eval(String input) throws IllegalStateException {
32+
return null;
33+
}
34+
35+
@Override
36+
public void close() { }
37+
}

‎java/ql/test/experimental/stubs/jshell/jdk/jshell/Snippet.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package jdk.jshell;
2+
3+
public abstract class Snippet {
4+
5+
public enum Kind {
6+
7+
IMPORT(true),
8+
9+
TYPE_DECL(true),
10+
11+
METHOD(true),
12+
13+
VAR(true),
14+
15+
EXPRESSION(false),
16+
17+
STATEMENT(false),
18+
19+
ERRONEOUS(false);
20+
21+
private final boolean isPersistent;
22+
23+
Kind(boolean isPersistent) {
24+
this.isPersistent = isPersistent;
25+
}
26+
27+
public boolean isPersistent() {
28+
return false;
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)