Merge pull request github#5467 from p0wn4j/groovy-execute

[Java] CWE-094: Query to detect Groovy Code Injections

Author: smowton

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

@@ -0,0 +1,81 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Apache Groovy is a powerful, optionally typed and dynamic language, 
+with static-typing and static compilation capabilities.
+
+It integrates smoothly with any Java program, 
+and immediately delivers to your application powerful features, 
+including scripting capabilities, Domain-Specific Language authoring, 
+runtime and compile-time meta-programming and functional programming.
+
+If a Groovy script is built using attacker-controlled data, 
+and then evaluated, then it may allow the attacker to achieve RCE.
+</p>
+</overview>
+
+<recommendation>
+<p>
+It is generally recommended to avoid using untrusted input in a Groovy evaluation.
+If this is not possible, use a sandbox solution. Developers must also take care that Groovy
+compile-time metaprogramming can also lead to RCE: it is possible to achieve RCE by compiling
+a Groovy script (see the article "Abusing Meta Programming for Unauthenticated RCE!" linked below).
+
+Groovy's <code>SecureASTCustomizer</code> allows securing source code by controlling what code constructs are permitted. 
+This is typically done when using Groovy for its scripting or domain specific language (DSL) features.
+The fundamental problem is that Groovy is a dynamic language, yet <code>SecureASTCustomizer</code> works by looking at Groovy AST statically.
+
+This makes it very easy for an attacker to bypass many of the intended checks
+(see https://kohsuke.org/2012/04/27/groovy-secureastcustomizer-is-harmful/).
+Therefore, besides <code>SecureASTCustomizer</code>, runtime checks are also necessary before calling Groovy methods
+(see https://melix.github.io/blog/2015/03/sandboxing.html).
+
+It is also possible to use a block-list method, excluding unwanted classes from being loaded by the JVM. 
+This method is not always recommended, because block-lists can be bypassed by unexpected values.
+
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example uses untrusted data to evaluate a Groovy script.
+</p>
+<sample src="GroovyInjectionBad.java" />
+
+<p>
+The following example uses classloader block-list approach to exclude loading dangerous classes.
+</p>
+<sample src="GroovyInjectionBlocklist.java" />
+
+</example>
+
+<references>
+<li>
+  Orange Tsai:
+  <a href="https://blog.orange.tw/2019/02/abusing-meta-programming-for-unauthenticated-rce.html">Abusing Meta Programming for Unauthenticated RCE!</a>.
+</li>
+<li>
+  Cédric Champeau:
+  <a href="https://melix.github.io/blog/2015/03/sandboxing.html">Improved sandboxing of Groovy scripts</a>.
+</li>
+<li>
+  Kohsuke Kawaguchi:
+  <a href="https://kohsuke.org/2012/04/27/groovy-secureastcustomizer-is-harmful/">Groovy SecureASTCustomizer is harmful</a>.
+</li>
+<li>
+  Welk1n:
+  <a href="https://github.com/welk1n/exploiting-groovy-in-Java/">Groovy Injection payloads</a>.
+</li>
+<li>
+  Charles Chan:
+  <a href="https://levelup.gitconnected.com/secure-groovy-script-execution-in-a-sandbox-ea39f80ee87/">Secure Groovy Script Execution in a Sandbox</a>.
+</li>
+<li>
+  Eugene:
+  <a href="https://stringconcat.com/en/scripting-and-sandboxing/">Scripting and sandboxing in a JVM environment</a>.
+</li>
+</references>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-094/GroovyInjection.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Groovy Language injection
+ * @description Evaluation of a user-controlled Groovy script
+ *              may lead to arbitrary code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/groovy-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import DataFlow::PathGraph
+import GroovyInjectionLib
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, GroovyInjectionConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Groovy Injection from $@.", source.getNode(),
+  "this user input"

java/ql/src/experimental/Security/CWE/CWE-094/GroovyInjectionBad.java

@@ -0,0 +1,27 @@
+public class GroovyInjection {
+    void injectionViaClassLoader(HttpServletRequest request) {    
+        String script = request.getParameter("script");
+        final GroovyClassLoader classLoader = new GroovyClassLoader();
+        Class groovy = classLoader.parseClass(script);
+        GroovyObject groovyObj = (GroovyObject) groovy.newInstance();
+    }
+
+    void injectionViaEval(HttpServletRequest request) {
+        String script = request.getParameter("script");
+        Eval.me(script);
+    }
+
+    void injectionViaGroovyShell(HttpServletRequest request) {
+        GroovyShell shell = new GroovyShell();
+        String script = request.getParameter("script");
+        shell.evaluate(script);
+    }
+
+    void injectionViaGroovyShellGroovyCodeSource(HttpServletRequest request) {
+        GroovyShell shell = new GroovyShell();
+        String script = request.getParameter("script");
+        GroovyCodeSource gcs = new GroovyCodeSource(script, "test", "Test");
+        shell.evaluate(gcs);
+    }
+}
+

java/ql/src/experimental/Security/CWE/CWE-094/GroovyInjectionBlocklist.java

@@ -0,0 +1,17 @@
+public class SandboxGroovyClassLoader extends ClassLoader {
+    public SandboxGroovyClassLoader(ClassLoader parent) {
+        super(parent);
+    }
+
+    /* override `loadClass` here to prevent loading sensitive classes, such as `java.lang.Runtime`, `java.lang.ProcessBuilder`, `java.lang.System`, etc.  */
+    /* Note we must also block `groovy.transform.ASTTest`, `groovy.lang.GrabConfig` and `org.buildobjects.process.ProcBuilder` to prevent compile-time RCE. */
+
+    static void runWithSandboxGroovyClassLoader() throws Exception {
+        // GOOD: route all class-loading via sand-boxing classloader.
+        SandboxGroovyClassLoader classLoader = new GroovyClassLoader(new SandboxGroovyClassLoader());
+        
+        Class<?> scriptClass = classLoader.parseClass(untrusted.getQueryString());
+        Object scriptInstance = scriptClass.newInstance();
+        Object result = scriptClass.getDeclaredMethod("bar", new Class[]{}).invoke(scriptInstance, new Object[]{});
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-094/GroovyInjectionLib.qll

@@ -0,0 +1,160 @@
+/**
+ * Provides classes and predicates for Groovy Code Injection
+ * taint-tracking configuration.
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+
+/** A data flow sink for Groovy expression injection vulnerabilities. */
+abstract private class GroovyInjectionSink extends DataFlow::ExprNode { }
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to evaluate a Groovy expression.
+ */
+class GroovyInjectionConfig extends TaintTracking::Configuration {
+  GroovyInjectionConfig() { this = "GroovyInjectionConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
+    groovyCodeSourceTaintStep(fromNode, toNode)
+  }
+}
+
+/** The class `groovy.lang.GroovyShell`. */
+private class TypeGroovyShell extends RefType {
+  TypeGroovyShell() { this.hasQualifiedName("groovy.lang", "GroovyShell") }
+}
+
+/** The class `groovy.lang.GroovyCodeSource`. */
+private class TypeGroovyCodeSource extends RefType {
+  TypeGroovyCodeSource() { this.hasQualifiedName("groovy.lang", "GroovyCodeSource") }
+}
+
+/**
+ * Methods in the `GroovyShell` class that evaluate a Groovy expression.
+ */
+private class GroovyShellMethod extends Method {
+  GroovyShellMethod() {
+    this.getDeclaringType() instanceof TypeGroovyShell and
+    this.getName() in ["evaluate", "parse", "run"]
+  }
+}
+
+private class GroovyShellMethodAccess extends MethodAccess {
+  GroovyShellMethodAccess() { this.getMethod() instanceof GroovyShellMethod }
+}
+
+/**
+ * Holds if `fromNode` to `toNode` is a dataflow step from a tainted string to
+ * a `GroovyCodeSource` instance, i.e. `new GroovyCodeSource(tainted, ...)`.
+ */
+private predicate groovyCodeSourceTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
+  exists(ConstructorCall gcscc |
+    gcscc.getConstructedType() instanceof TypeGroovyCodeSource and
+    gcscc = toNode.asExpr() and
+    gcscc.getArgument(0) = fromNode.asExpr()
+  )
+}
+
+/**
+ * A sink for Groovy Injection via the `GroovyShell` class.
+ *
+ * ```
+ * GroovyShell gs = new GroovyShell();
+ * gs.evaluate(sink, ....)
+ * gs.run(sink, ....)
+ * gs.parse(sink,...)
+ * ```
+ */
+private class GroovyShellSink extends GroovyInjectionSink {
+  GroovyShellSink() {
+    exists(GroovyShellMethodAccess ma, Argument firstArg |
+      ma.getArgument(0) = firstArg and
+      firstArg = this.asExpr() and
+      (
+        firstArg.getType() instanceof TypeString or
+        firstArg.getType() instanceof TypeGroovyCodeSource
+      )
+    )
+  }
+}
+
+/** The class `groovy.util.Eval`. */
+private class TypeEval extends RefType {
+  TypeEval() { this.hasQualifiedName("groovy.util", "Eval") }
+}
+
+/**
+ * Methods in the `Eval` class that evaluate a Groovy expression.
+ */
+private class EvalMethod extends Method {
+  EvalMethod() {
+    this.getDeclaringType() instanceof TypeEval and
+    this.getName() in ["me", "x", "xy", "xyz"]
+  }
+}
+
+private class EvalMethodAccess extends MethodAccess {
+  EvalMethodAccess() { this.getMethod() instanceof EvalMethod }
+
+  Expr getArgumentExpr() { result = this.getArgument(this.getNumArgument() - 1) }
+}
+
+/**
+ * A sink for Groovy Injection via the `Eval` class.
+ *
+ * ```
+ * Eval.me(sink)
+ * Eval.me("p1", "p2", sink)
+ * Eval.x("p1", sink)
+ * Eval.xy("p1", "p2" sink)
+ * Eval.xyz("p1", "p2", "p3", sink)
+ * ```
+ */
+private class EvalSink extends GroovyInjectionSink {
+  EvalSink() { exists(EvalMethodAccess ma | ma.getArgumentExpr() = this.asExpr()) }
+}
+
+/** The class `groovy.lang.GroovyClassLoader`. */
+private class TypeGroovyClassLoader extends RefType {
+  TypeGroovyClassLoader() { this.hasQualifiedName("groovy.lang", "GroovyClassLoader") }
+}
+
+/**
+ * A method in the `GroovyClassLoader` class that evaluates a Groovy expression.
+ */
+private class GroovyClassLoaderParseClassMethod extends Method {
+  GroovyClassLoaderParseClassMethod() {
+    this.getDeclaringType() instanceof TypeGroovyClassLoader and
+    this.hasName("parseClass")
+  }
+}
+
+private class GroovyClassLoaderParseClassMethodAccess extends MethodAccess {
+  GroovyClassLoaderParseClassMethodAccess() {
+    this.getMethod() instanceof GroovyClassLoaderParseClassMethod
+  }
+}
+
+/**
+ * A sink for Groovy Injection via the `GroovyClassLoader` class.
+ *
+ * ```
+ * GroovyClassLoader classLoader = new GroovyClassLoader();
+ * Class groovy = classLoader.parseClass(script);
+ * ```
+ *
+ * Groovy supports compile-time metaprogramming, so just calling the `parseClass`
+ * method is enough to achieve RCE.
+ */
+private class GroovyClassLoadParseClassSink extends GroovyInjectionSink {
+  GroovyClassLoadParseClassSink() {
+    exists(GroovyClassLoaderParseClassMethodAccess ma | ma.getArgument(0) = this.asExpr())
+  }
+}

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

@@ -0,0 +1,39 @@
+import groovy.lang.GroovyClassLoader;
+import groovy.lang.GroovyCodeSource;
+import groovy.lang.GroovyObject;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+
+public class GroovyClassLoaderTest extends HttpServlet {
+
+    protected void doGet(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        try {
+            String script = request.getParameter("script");
+            final GroovyClassLoader classLoader = new GroovyClassLoader();
+            Class groovy = classLoader.parseClass(script);
+            GroovyObject groovyObj = (GroovyObject) groovy.newInstance();
+
+        } catch (Exception e) {
+            // Ignore
+        }
+    }
+
+    protected void doPost(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        try {
+            String script = request.getParameter("script");
+            final GroovyClassLoader classLoader = new GroovyClassLoader();
+            GroovyCodeSource gcs = new GroovyCodeSource(script, "test", "Test");
+            Class groovy = classLoader.parseClass(gcs);
+            GroovyObject groovyObj = (GroovyObject) groovy.newInstance();
+        } catch (Exception e) {
+            // Ignore
+        }
+    }
+}
+

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

@@ -0,0 +1,41 @@
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import groovy.util.Eval;
+
+public class GroovyEvalTest extends HttpServlet {
+
+    protected void doGet(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        String script = request.getParameter("script");
+        Eval.me(script);
+    }
+
+    protected void doPost(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        String script = request.getParameter("script");
+        Eval.me("test", "result", script);
+    }
+
+    protected void doPut(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        String script = request.getParameter("script");
+        Eval.x("result2", script);
+
+    }
+
+    protected void doDelete(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        String script = request.getParameter("script");
+        Eval.xy("result3", "result4", script);
+    }
+
+    protected void doPatch(HttpServletRequest request, HttpServletResponse response)
+            throws ServletException, IOException {
+        String script = request.getParameter("script");
+        Eval.xyz("result3", "result4", "aaa", script);
+    }
+}
+