Merge pull request github#5957 from haby0/java/BeanShellInjection

smowton · web-flow · commit 6302187a5dec · 2021-06-18T12:38:51.000+01:00
Java: BeanShell Injection
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.java b/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.java
@@ -0,0 +1,33 @@
+import bsh.Interpreter;
+import javax.servlet.http.HttpServletRequest;
+import org.springframework.scripting.bsh.BshScriptEvaluator;
+import org.springframework.scripting.support.StaticScriptSource;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+
+@Controller
+public class BeanShellInjection {
+
+	@GetMapping(value = "bad1")
+	public void bad1(HttpServletRequest request) {
+		String code = request.getParameter("code");
+		BshScriptEvaluator evaluator = new BshScriptEvaluator();
+		evaluator.evaluate(new StaticScriptSource(code)); //bad
+	}
+
+	@GetMapping(value = "bad2")
+	public void bad2(HttpServletRequest request) throws Exception {
+		String code = request.getParameter("code");
+		Interpreter interpreter = new Interpreter();
+		interpreter.eval(code);  //bad
+	}
+
+	@GetMapping(value = "bad3")
+	public void bad3(HttpServletRequest request) {
+		String code = request.getParameter("code");
+		StaticScriptSource staticScriptSource = new StaticScriptSource("test");
+		staticScriptSource.setScript(code);
+		BshScriptEvaluator evaluator = new BshScriptEvaluator();
+		evaluator.evaluate(staticScriptSource);  //bad
+	}
+}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.qhelp
@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+BeanShell is a small, free, embeddable Java source interpreter with object scripting language 
+features, written in Java. BeanShell dynamically executes standard Java syntax and extends it 
+with common scripting conveniences such as loose types, commands, and method closures like 
+those in Perl and JavaScript. If a BeanShell expression is built using attacker-controlled data, 
+and then evaluated, then it may allow the attacker to run arbitrary code.
+</p>
+</overview>
+
+<recommendation>
+<p>
+It is generally recommended to avoid using untrusted input in a BeanShell expression.
+If it is not possible, BeanShell expressions should be run in a sandbox that allows accessing only
+explicitly allowed classes.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example uses untrusted data to build and run a BeanShell expression.
+</p>
+<sample src="BeanShellInjection.java" />
+</example>
+
+<references>
+<li>
+CVE-2016-2510:<a href="https://nvd.nist.gov/vuln/detail/CVE-2016-2510">BeanShell Injection</a>.
+</li>
+</references>
+</qhelp>
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.ql
@@ -0,0 +1,47 @@
+/**
+ * @name BeanShell injection
+ * @description Evaluation of a user-controlled BeanShell expression
+ *              may lead to arbitrary code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/beanshell-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import BeanShellInjection
+import semmle.code.java.dataflow.FlowSources
+import DataFlow::PathGraph
+
+class BeanShellInjectionConfig extends TaintTracking::Configuration {
+  BeanShellInjectionConfig() { this = "BeanShellInjectionConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof BeanShellInjectionSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) {
+    exists(ClassInstanceExpr cie |
+      cie.getConstructedType()
+          .hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and
+      cie.getArgument(0) = prod.asExpr() and
+      cie = succ.asExpr()
+    )
+    or
+    exists(MethodAccess ma |
+      ma.getMethod().hasName("setScript") and
+      ma.getMethod()
+          .getDeclaringType()
+          .hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and
+      ma.getArgument(0) = prod.asExpr() and
+      ma.getQualifier() = succ.asExpr()
+    )
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, BeanShellInjectionConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "BeanShell injection from $@.", source.getNode(),
+  "this user input"
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.qll b/java/ql/src/experimental/Security/CWE/CWE-094/BeanShellInjection.qll
@@ -0,0 +1,28 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+
+/** A call to `Interpreter.eval`. */
+class InterpreterEvalCall extends MethodAccess {
+  InterpreterEvalCall() {
+    this.getMethod().hasName("eval") and
+    this.getMethod().getDeclaringType().hasQualifiedName("bsh", "Interpreter")
+  }
+}
+
+/** A call to `BshScriptEvaluator.evaluate`. */
+class BshScriptEvaluatorEvaluateCall extends MethodAccess {
+  BshScriptEvaluatorEvaluateCall() {
+    this.getMethod().hasName("evaluate") and
+    this.getMethod()
+        .getDeclaringType()
+        .hasQualifiedName("org.springframework.scripting.bsh", "BshScriptEvaluator")
+  }
+}
+
+/** A sink for BeanShell expression injection vulnerabilities. */
+class BeanShellInjectionSink extends DataFlow::Node {
+  BeanShellInjectionSink() {
+    this.asExpr() = any(InterpreterEvalCall iec).getArgument(0) or
+    this.asExpr() = any(BshScriptEvaluatorEvaluateCall bseec).getArgument(0)
+  }
+}
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/BeanShellInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/BeanShellInjection.expected
@@ -0,0 +1,15 @@
+edges
+| BeanShellInjection.java:13:17:13:44 | getParameter(...) : String | BeanShellInjection.java:15:22:15:49 | new StaticScriptSource(...) |
+| BeanShellInjection.java:20:17:20:44 | getParameter(...) : String | BeanShellInjection.java:22:20:22:23 | code |
+| BeanShellInjection.java:27:17:27:44 | getParameter(...) : String | BeanShellInjection.java:31:22:31:39 | staticScriptSource |
+nodes
+| BeanShellInjection.java:13:17:13:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| BeanShellInjection.java:15:22:15:49 | new StaticScriptSource(...) | semmle.label | new StaticScriptSource(...) |
+| BeanShellInjection.java:20:17:20:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| BeanShellInjection.java:22:20:22:23 | code | semmle.label | code |
+| BeanShellInjection.java:27:17:27:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| BeanShellInjection.java:31:22:31:39 | staticScriptSource | semmle.label | staticScriptSource |
+#select
+| 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 |
+| 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 |
+| 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 |
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/BeanShellInjection.java b/java/ql/test/experimental/query-tests/security/CWE-094/BeanShellInjection.java
@@ -0,0 +1,33 @@
+import bsh.Interpreter;
+import javax.servlet.http.HttpServletRequest;
+import org.springframework.scripting.bsh.BshScriptEvaluator;
+import org.springframework.scripting.support.StaticScriptSource;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+
+@Controller
+public class BeanShellInjection {
+
+	@GetMapping(value = "bad1")
+	public void bad1(HttpServletRequest request) {
+		String code = request.getParameter("code");
+		BshScriptEvaluator evaluator = new BshScriptEvaluator();
+		evaluator.evaluate(new StaticScriptSource(code)); //bad
+	}
+
+	@GetMapping(value = "bad2")
+	public void bad2(HttpServletRequest request) throws Exception {
+		String code = request.getParameter("code");
+		Interpreter interpreter = new Interpreter();
+		interpreter.eval(code);  //bad
+	}
+
+	@GetMapping(value = "bad3")
+	public void bad3(HttpServletRequest request) {
+		String code = request.getParameter("code");
+		StaticScriptSource staticScriptSource = new StaticScriptSource("test");
+		staticScriptSource.setScript(code);
+		BshScriptEvaluator evaluator = new BshScriptEvaluator();
+		evaluator.evaluate(staticScriptSource);  //bad
+	}
+}
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/BeanShellInjection.qlref b/java/ql/test/experimental/query-tests/security/CWE-094/BeanShellInjection.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-094/BeanShellInjection.ql
diff --git 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
+//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
diff --git a/java/ql/test/stubs/bsh-2.0b5/bsh/ConsoleInterface.java b/java/ql/test/stubs/bsh-2.0b5/bsh/ConsoleInterface.java
@@ -0,0 +1,18 @@
+package bsh;
+
+import java.io.PrintStream;
+import java.io.Reader;
+
+public interface ConsoleInterface {
+    Reader getIn();
+
+    PrintStream getOut();
+
+    PrintStream getErr();
+
+    void println(Object var1);
+
+    void print(Object var1);
+
+    void error(Object var1);
+}
diff --git a/java/ql/test/stubs/bsh-2.0b5/bsh/EvalError.java b/java/ql/test/stubs/bsh-2.0b5/bsh/EvalError.java
@@ -0,0 +1,5 @@
+package bsh;
+
+public class EvalError extends Exception {
+    
+}
diff --git a/java/ql/test/stubs/bsh-2.0b5/bsh/Interpreter.java b/java/ql/test/stubs/bsh-2.0b5/bsh/Interpreter.java
@@ -0,0 +1,125 @@
+package bsh;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.ObjectInputStream;
+import java.io.PrintStream;
+import java.io.Reader;
+import java.io.Serializable;
+import java.io.StringReader;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+public class Interpreter implements Runnable, ConsoleInterface, Serializable {
+
+    public Interpreter(Reader in, PrintStream out, PrintStream err, boolean interactive, NameSpace namespace, Interpreter parent, String sourceFileInfo) { }
+
+    public Interpreter(Reader in, PrintStream out, PrintStream err, boolean interactive, NameSpace namespace) { }
+
+    public Interpreter(Reader in, PrintStream out, PrintStream err, boolean interactive) { }
+
+    public Interpreter(ConsoleInterface console, NameSpace globalNameSpace) { }
+
+    public Interpreter(ConsoleInterface console) { }
+
+    public Interpreter() { }
+
+    public void setConsole(ConsoleInterface console) { }
+
+    private void initRootSystemObject() { }
+
+    public void setNameSpace(NameSpace globalNameSpace) { }
+
+    public NameSpace getNameSpace() {
+        return null;
+    }
+
+    public static void main(String[] args) { }
+
+    public static void invokeMain(Class clas, String[] args) throws Exception { }
+
+    public void run() { }
+
+    public Object source(String filename, NameSpace nameSpace) throws FileNotFoundException, IOException, EvalError {
+        return null;
+    }
+
+    public Object source(String filename) throws FileNotFoundException, IOException, EvalError {
+        return null;
+    }
+
+    public Object eval(Reader in, NameSpace nameSpace, String sourceFileInfo) throws EvalError {
+        return null;
+    }
+
+    public Object eval(Reader in) throws EvalError {
+        return null;
+    }
+
+    public Object eval(String statements) throws EvalError {
+        return null;
+    }
+
+    public Object eval(String statements, NameSpace nameSpace) throws EvalError {
+        return null;
+    }
+
+    private String showEvalString(String s) {
+        return null;
+    }
+
+    public final void error(Object o) { }
+
+    public Reader getIn() {
+        return null;
+    }
+
+    public PrintStream getOut() {
+        return null;
+    }
+
+    public PrintStream getErr() {
+        return null;
+    }
+
+    public final void println(Object o) { }
+
+    public final void print(Object o) { }
+
+    public static final void debug(String s) { }
+
+    public Object get(String name) throws EvalError {
+        return null;
+    }
+
+    Object getu(String name) {
+        return null;
+    }
+
+    public void set(String name, Object value) throws EvalError { }
+
+    void setu(String name, Object value) { }
+
+    public void set(String name, long value) throws EvalError { }
+
+    public void set(String name, int value) throws EvalError { }
+
+    public void set(String name, double value) throws EvalError { }
+
+    public void set(String name, float value) throws EvalError { }
+
+    public void set(String name, boolean value) throws EvalError { }
+
+    public void unset(String name) throws EvalError { }
+
+    public Object getInterface(Class interf) throws EvalError {
+        return null;
+    }
+}
diff --git a/java/ql/test/stubs/bsh-2.0b5/bsh/NameSpace.java b/java/ql/test/stubs/bsh-2.0b5/bsh/NameSpace.java
@@ -0,0 +1,7 @@
+package bsh;
+
+import java.io.Serializable;
+
+public class NameSpace implements Serializable {
+
+}
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/Aware.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/Aware.java
@@ -0,0 +1,4 @@
+package org.springframework.beans.factory;
+
+public interface Aware {
+}
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/BeanClassLoaderAware.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/BeanClassLoaderAware.java
@@ -0,0 +1,5 @@
+package org.springframework.beans.factory;
+
+public interface BeanClassLoaderAware extends Aware {
+    void setBeanClassLoader(ClassLoader var1);
+}
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/ScriptEvaluator.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/ScriptEvaluator.java
@@ -0,0 +1,12 @@
+package org.springframework.scripting;
+
+import java.util.Map;
+import org.springframework.lang.Nullable;
+
+public interface ScriptEvaluator {
+    @Nullable
+    Object evaluate(ScriptSource var1) ;
+
+    @Nullable
+    Object evaluate(ScriptSource var1, @Nullable Map<String, Object> var2) ;
+}
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/ScriptSource.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/ScriptSource.java
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/bsh/BshScriptEvaluator.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/bsh/BshScriptEvaluator.java
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/support/StaticScriptSource.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/scripting/support/StaticScriptSource.java

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+experimental/Security/CWE/CWE-094/BeanShellInjection.ql`
Original file line number	Diff line number	Diff 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
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +package bsh;
++
 +public class EvalError extends Exception {
++
 +}