Skip to content

Commit 6302187

Browse files
smowtonsmowton
authored
Merge pull request #5957 from haby0/java/BeanShellInjection
Java: BeanShell Injection
2 parents f829fff + a73cb3f commit 6302187

18 files changed

Lines changed: 437 additions & 1 deletion

File tree

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import bsh.Interpreter;
2+
import javax.servlet.http.HttpServletRequest;
3+
import org.springframework.scripting.bsh.BshScriptEvaluator;
4+
import org.springframework.scripting.support.StaticScriptSource;
5+
import org.springframework.stereotype.Controller;
6+
import org.springframework.web.bind.annotation.GetMapping;
7+
8+
@Controller
9+
public class BeanShellInjection {
10+
11+
@GetMapping(value = "bad1")
12+
public void bad1(HttpServletRequest request) {
13+
String code = request.getParameter("code");
14+
BshScriptEvaluator evaluator = new BshScriptEvaluator();
15+
evaluator.evaluate(new StaticScriptSource(code)); //bad
16+
}
17+
18+
@GetMapping(value = "bad2")
19+
public void bad2(HttpServletRequest request) throws Exception {
20+
String code = request.getParameter("code");
21+
Interpreter interpreter = new Interpreter();
22+
interpreter.eval(code); //bad
23+
}
24+
25+
@GetMapping(value = "bad3")
26+
public void bad3(HttpServletRequest request) {
27+
String code = request.getParameter("code");
28+
StaticScriptSource staticScriptSource = new StaticScriptSource("test");
29+
staticScriptSource.setScript(code);
30+
BshScriptEvaluator evaluator = new BshScriptEvaluator();
31+
evaluator.evaluate(staticScriptSource); //bad
32+
}
33+
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
BeanShell is a small, free, embeddable Java source interpreter with object scripting language
7+
features, written in Java. BeanShell dynamically executes standard Java syntax and extends it
8+
with common scripting conveniences such as loose types, commands, and method closures like
9+
those in Perl and JavaScript. If a BeanShell expression is built using attacker-controlled data,
10+
and then evaluated, then it may allow the attacker to run arbitrary code.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
It is generally recommended to avoid using untrusted input in a BeanShell expression.
17+
If it is not possible, BeanShell expressions should be run in a sandbox that allows accessing only
18+
explicitly allowed classes.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
The following example uses untrusted data to build and run a BeanShell expression.
25+
</p>
26+
<sample src="BeanShellInjection.java" />
27+
</example>
28+
29+
<references>
30+
<li>
31+
CVE-2016-2510:<a href="https://nvd.nist.gov/vuln/detail/CVE-2016-2510">BeanShell Injection</a>.
32+
</li>
33+
</references>
34+
</qhelp>

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* @name BeanShell injection
3+
* @description Evaluation of a user-controlled BeanShell expression
4+
* may lead to arbitrary code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/beanshell-injection
9+
* @tags security
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import java
14+
import BeanShellInjection
15+
import semmle.code.java.dataflow.FlowSources
16+
import DataFlow::PathGraph
17+
18+
class BeanShellInjectionConfig extends TaintTracking::Configuration {
19+
BeanShellInjectionConfig() { this = "BeanShellInjectionConfig" }
20+
21+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
22+
23+
override predicate isSink(DataFlow::Node sink) { sink instanceof BeanShellInjectionSink }
24+
25+
override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) {
26+
exists(ClassInstanceExpr cie |
27+
cie.getConstructedType()
28+
.hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and
29+
cie.getArgument(0) = prod.asExpr() and
30+
cie = succ.asExpr()
31+
)
32+
or
33+
exists(MethodAccess ma |
34+
ma.getMethod().hasName("setScript") and
35+
ma.getMethod()
36+
.getDeclaringType()
37+
.hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and
38+
ma.getArgument(0) = prod.asExpr() and
39+
ma.getQualifier() = succ.asExpr()
40+
)
41+
}
42+
}
43+
44+
from DataFlow::PathNode source, DataFlow::PathNode sink, BeanShellInjectionConfig conf
45+
where conf.hasFlowPath(source, sink)
46+
select sink.getNode(), source, sink, "BeanShell injection from $@.", source.getNode(),
47+
"this user input"

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
4+
/** A call to `Interpreter.eval`. */
5+
class InterpreterEvalCall extends MethodAccess {
6+
InterpreterEvalCall() {
7+
this.getMethod().hasName("eval") and
8+
this.getMethod().getDeclaringType().hasQualifiedName("bsh", "Interpreter")
9+
}
10+
}
11+
12+
/** A call to `BshScriptEvaluator.evaluate`. */
13+
class BshScriptEvaluatorEvaluateCall extends MethodAccess {
14+
BshScriptEvaluatorEvaluateCall() {
15+
this.getMethod().hasName("evaluate") and
16+
this.getMethod()
17+
.getDeclaringType()
18+
.hasQualifiedName("org.springframework.scripting.bsh", "BshScriptEvaluator")
19+
}
20+
}
21+
22+
/** A sink for BeanShell expression injection vulnerabilities. */
23+
class BeanShellInjectionSink extends DataFlow::Node {
24+
BeanShellInjectionSink() {
25+
this.asExpr() = any(InterpreterEvalCall iec).getArgument(0) or
26+
this.asExpr() = any(BshScriptEvaluatorEvaluateCall bseec).getArgument(0)
27+
}
28+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
edges
2+
| BeanShellInjection.java:13:17:13:44 | getParameter(...) : String | BeanShellInjection.java:15:22:15:49 | new StaticScriptSource(...) |
3+
| BeanShellInjection.java:20:17:20:44 | getParameter(...) : String | BeanShellInjection.java:22:20:22:23 | code |
4+
| BeanShellInjection.java:27:17:27:44 | getParameter(...) : String | BeanShellInjection.java:31:22:31:39 | staticScriptSource |
5+
nodes
6+
| BeanShellInjection.java:13:17:13:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
7+
| BeanShellInjection.java:15:22:15:49 | new StaticScriptSource(...) | semmle.label | new StaticScriptSource(...) |
8+
| BeanShellInjection.java:20:17:20:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
9+
| BeanShellInjection.java:22:20:22:23 | code | semmle.label | code |
10+
| BeanShellInjection.java:27:17:27:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
11+
| BeanShellInjection.java:31:22:31:39 | staticScriptSource | semmle.label | staticScriptSource |
12+
#select
13+
| BeanShellInjection.java:15:22:15:49 | new StaticScriptSource(...) | BeanShellInjection.java:13:17:13:44 | getParameter(...) : String | BeanShellInjection.java:15:22:15:49 | new StaticScriptSource(...) | BeanShell injection from $@. | BeanShellInjection.java:13:17:13:44 | getParameter(...) | this user input |
14+
| BeanShellInjection.java:22:20:22:23 | code | BeanShellInjection.java:20:17:20:44 | getParameter(...) : String | BeanShellInjection.java:22:20:22:23 | code | BeanShell injection from $@. | BeanShellInjection.java:20:17:20:44 | getParameter(...) | this user input |
15+
| BeanShellInjection.java:31:22:31:39 | staticScriptSource | BeanShellInjection.java:27:17:27:44 | getParameter(...) : String | BeanShellInjection.java:31:22:31:39 | staticScriptSource | BeanShell injection from $@. | BeanShellInjection.java:27:17:27:44 | getParameter(...) | this user input |

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import bsh.Interpreter;
2+
import javax.servlet.http.HttpServletRequest;
3+
import org.springframework.scripting.bsh.BshScriptEvaluator;
4+
import org.springframework.scripting.support.StaticScriptSource;
5+
import org.springframework.stereotype.Controller;
6+
import org.springframework.web.bind.annotation.GetMapping;
7+
8+
@Controller
9+
public class BeanShellInjection {
10+
11+
@GetMapping(value = "bad1")
12+
public void bad1(HttpServletRequest request) {
13+
String code = request.getParameter("code");
14+
BshScriptEvaluator evaluator = new BshScriptEvaluator();
15+
evaluator.evaluate(new StaticScriptSource(code)); //bad
16+
}
17+
18+
@GetMapping(value = "bad2")
19+
public void bad2(HttpServletRequest request) throws Exception {
20+
String code = request.getParameter("code");
21+
Interpreter interpreter = new Interpreter();
22+
interpreter.eval(code); //bad
23+
}
24+
25+
@GetMapping(value = "bad3")
26+
public void bad3(HttpServletRequest request) {
27+
String code = request.getParameter("code");
28+
StaticScriptSource staticScriptSource = new StaticScriptSource("test");
29+
staticScriptSource.setScript(code);
30+
BshScriptEvaluator evaluator = new BshScriptEvaluator();
31+
evaluator.evaluate(staticScriptSource); //bad
32+
}
33+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-094/BeanShellInjection.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
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

‎java/ql/test/stubs/bsh-2.0b5/bsh/ConsoleInterface.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package bsh;
2+
3+
import java.io.PrintStream;
4+
import java.io.Reader;
5+
6+
public interface ConsoleInterface {
7+
Reader getIn();
8+
9+
PrintStream getOut();
10+
11+
PrintStream getErr();
12+
13+
void println(Object var1);
14+
15+
void print(Object var1);
16+
17+
void error(Object var1);
18+
}

‎java/ql/test/stubs/bsh-2.0b5/bsh/EvalError.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package bsh;
2+
3+
public class EvalError extends Exception {
4+
5+
}

0 commit comments

Comments
 (0)