mirror of
https://github.com/github/codeql.git
synced 2025-12-21 11:16:30 +01:00
Merge pull request #6062 from atorralba/atorralba/promote-groovy-injection
Java: Promote Groovy Code Injection from experimental
This commit is contained in:
Anders Schack-Mulligen
@@ -28,9 +28,9 @@ This is typically done when using Groovy for its scripting or domain specific la
|
||||
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/).
|
||||
(see [Groovy SecureASTCustomizer is harmful](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).
|
||||
(see [Improved sandboxing of Groovy scripts](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.
|
||||
@@ -11,8 +11,8 @@
|
||||
*/
|
||||
|
||||
import java
|
||||
import semmle.code.java.security.GroovyInjectionQuery
|
||||
import DataFlow::PathGraph
|
||||
import GroovyInjectionLib
|
||||
|
||||
from DataFlow::PathNode source, DataFlow::PathNode sink, GroovyInjectionConfig conf
|
||||
where conf.hasFlowPath(source, sink)
|
||||
@@ -1,160 +0,0 @@
|
||||
/**
|
||||
* 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())
|
||||
}
|
||||
}
|
||||
@@ -97,6 +97,7 @@ private module Frameworks {
|
||||
private import semmle.code.java.frameworks.spring.SpringWebMultipart
|
||||
private import semmle.code.java.security.ResponseSplitting
|
||||
private import semmle.code.java.security.InformationLeak
|
||||
private import semmle.code.java.security.GroovyInjection
|
||||
private import semmle.code.java.security.JexlInjectionSinkModels
|
||||
private import semmle.code.java.security.LdapInjection
|
||||
private import semmle.code.java.security.MvelInjection
|
||||
@@ -329,6 +330,7 @@ private predicate summaryModelCsv(string row) {
|
||||
"java.io;File;false;File;;;Argument[0];Argument[-1];taint",
|
||||
"java.io;File;false;File;;;Argument[1];Argument[-1];taint",
|
||||
"java.net;URI;false;URI;(String);;Argument[0];Argument[-1];taint",
|
||||
"java.net;URL;false;URL;(String);;Argument[0];Argument[-1];taint",
|
||||
"javax.xml.transform.stream;StreamSource;false;StreamSource;;;Argument[0];Argument[-1];taint",
|
||||
"javax.xml.transform.sax;SAXSource;false;SAXSource;(InputSource);;Argument[0];Argument[-1];taint",
|
||||
"javax.xml.transform.sax;SAXSource;false;SAXSource;(XMLReader,InputSource);;Argument[1];Argument[-1];taint",
|
||||
|
||||
169
java/ql/src/semmle/code/java/security/GroovyInjection.qll
Normal file
169
java/ql/src/semmle/code/java/security/GroovyInjection.qll
Normal file
@@ -0,0 +1,169 @@
|
||||
/** Provides classes to reason about Groovy code injection attacks. */
|
||||
|
||||
private import semmle.code.java.dataflow.DataFlow
|
||||
private import semmle.code.java.dataflow.ExternalFlow
|
||||
private import semmle.code.java.frameworks.Networking
|
||||
|
||||
/** A data flow sink for Groovy expression injection vulnerabilities. */
|
||||
abstract class GroovyInjectionSink extends DataFlow::ExprNode { }
|
||||
|
||||
/**
|
||||
* A unit class for adding additional taint steps.
|
||||
*
|
||||
* Extend this class to add additional taint steps that should apply to the `GroovyInjectionConfig`.
|
||||
*/
|
||||
class GroovyInjectionAdditionalTaintStep extends Unit {
|
||||
/**
|
||||
* Holds if the step from `node1` to `node2` should be considered a taint
|
||||
* step for the `GroovyInjectionConfig` configuration.
|
||||
*/
|
||||
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
|
||||
}
|
||||
|
||||
private class DefaultGroovyInjectionSink extends GroovyInjectionSink {
|
||||
DefaultGroovyInjectionSink() { sinkNode(this, "groovy") }
|
||||
}
|
||||
|
||||
private class DefaultGroovyInjectionSinkModel extends SinkModelCsv {
|
||||
override predicate row(string row) {
|
||||
row =
|
||||
[
|
||||
// Signatures are specified to exclude sinks of the type `File`
|
||||
"groovy.lang;GroovyShell;false;evaluate;(GroovyCodeSource);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;evaluate;(Reader);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;evaluate;(Reader,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;evaluate;(String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;evaluate;(String,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;evaluate;(String,String,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;evaluate;(URI);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;parse;(Reader);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;parse;(Reader,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;parse;(String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;parse;(String,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;parse;(URI);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(GroovyCodeSource,String[]);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(GroovyCodeSource,List);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(Reader,String,String[]);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(Reader,String,List);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(String,String,String[]);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(String,String,List);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(URI,String[]);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyShell;false;run;(URI,List);;Argument[0];groovy",
|
||||
"groovy.util;Eval;false;me;(String);;Argument[0];groovy",
|
||||
"groovy.util;Eval;false;me;(String,Object,String);;Argument[2];groovy",
|
||||
"groovy.util;Eval;false;x;(Object,String);;Argument[1];groovy",
|
||||
"groovy.util;Eval;false;xy;(Object,Object,String);;Argument[2];groovy",
|
||||
"groovy.util;Eval;false;xyz;(Object,Object,Object,String);;Argument[3];groovy",
|
||||
"groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource,boolean);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyClassLoader;false;parseClass;(InputStream,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyClassLoader;false;parseClass;(Reader,String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyClassLoader;false;parseClass;(String);;Argument[0];groovy",
|
||||
"groovy.lang;GroovyClassLoader;false;parseClass;(String,String);;Argument[0];groovy",
|
||||
"org.codehaus.groovy.control;CompilationUnit;false;compile;;;Argument[-1];groovy"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
/** A set of additional taint steps to consider when taint tracking Groovy related data flows. */
|
||||
private class DefaultGroovyInjectionAdditionalTaintStep extends GroovyInjectionAdditionalTaintStep {
|
||||
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
groovyCodeSourceTaintStep(node1, node2) or
|
||||
groovyCompilationUnitTaintStep(node1, node2) or
|
||||
groovySourceUnitTaintStep(node1, node2) or
|
||||
groovyReaderSourceTaintStep(node1, node2)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `fromNode` to `toNode` is a dataflow step from a tainted string to
|
||||
* a `GroovyCodeSource` instance by calling `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()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `fromNode` to `toNode` is a dataflow step from a tainted object to
|
||||
* a `CompilationUnit` instance by calling `compilationUnit.addSource(..., tainted)`.
|
||||
*/
|
||||
private predicate groovyCompilationUnitTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
|
||||
exists(MethodAccess ma, Method m |
|
||||
ma.getMethod() = m and
|
||||
m.hasName("addSource") and
|
||||
m.getDeclaringType() instanceof TypeGroovyCompilationUnit
|
||||
|
|
||||
fromNode.asExpr() = ma.getArgument(ma.getNumArgument() - 1) and
|
||||
toNode.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = ma.getQualifier()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `fromNode` to `toNode` is a dataflow step from a tainted object to
|
||||
* a `SourceUnit` instance by calling `new SourceUnit(..., tainted, ...)`
|
||||
* or `SourceUnit.create(..., tainted)`
|
||||
*/
|
||||
private predicate groovySourceUnitTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
|
||||
exists(ClassInstanceExpr cie, Argument arg, int index |
|
||||
cie.getConstructedType() instanceof TypeGroovySourceUnit and
|
||||
arg = cie.getArgument(index) and
|
||||
(
|
||||
index = 0 and arg.getType() instanceof TypeUrl
|
||||
or
|
||||
index = 1 and
|
||||
(
|
||||
arg.getType() instanceof TypeString or
|
||||
arg.getType() instanceof TypeReaderSource
|
||||
)
|
||||
)
|
||||
|
|
||||
fromNode.asExpr() = arg and
|
||||
toNode.asExpr() = cie
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma, Method m |
|
||||
ma.getMethod() = m and
|
||||
m.hasName("create") and
|
||||
m.getDeclaringType() instanceof TypeGroovySourceUnit
|
||||
|
|
||||
fromNode.asExpr() = ma.getArgument(1) and toNode.asExpr() = ma
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `fromNode` to `toNode` is a dataflow step from a tainted object to
|
||||
* a `ReaderSource` instance by calling `new ReaderSource(tainted, ...)`.
|
||||
*/
|
||||
private predicate groovyReaderSourceTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
|
||||
exists(ClassInstanceExpr cie | cie.getConstructedType() instanceof TypeReaderSource |
|
||||
fromNode.asExpr() = cie.getArgument(0) and toNode.asExpr() = cie
|
||||
)
|
||||
}
|
||||
|
||||
/** The class `groovy.lang.GroovyCodeSource`. */
|
||||
private class TypeGroovyCodeSource extends RefType {
|
||||
TypeGroovyCodeSource() { this.hasQualifiedName("groovy.lang", "GroovyCodeSource") }
|
||||
}
|
||||
|
||||
/** The class `org.codehaus.groovy.control.CompilationUnit`. */
|
||||
private class TypeGroovyCompilationUnit extends RefType {
|
||||
TypeGroovyCompilationUnit() {
|
||||
this.hasQualifiedName("org.codehaus.groovy.control", "CompilationUnit")
|
||||
}
|
||||
}
|
||||
|
||||
/** The class `org.codehaus.groovy.control.CompilationUnit`. */
|
||||
private class TypeGroovySourceUnit extends RefType {
|
||||
TypeGroovySourceUnit() { this.hasQualifiedName("org.codehaus.groovy.control", "SourceUnit") }
|
||||
}
|
||||
|
||||
/** The class `org.codehaus.groovy.control.io.ReaderSource`. */
|
||||
private class TypeReaderSource extends RefType {
|
||||
TypeReaderSource() {
|
||||
this.getASupertype*().hasQualifiedName("org.codehaus.groovy.control.io", "ReaderSource")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,22 @@
|
||||
/** Provides taint tracking configurations relating to Groovy injection vulnerabilities. */
|
||||
|
||||
import java
|
||||
import semmle.code.java.dataflow.FlowSources
|
||||
import semmle.code.java.dataflow.TaintTracking
|
||||
import semmle.code.java.security.GroovyInjection
|
||||
|
||||
/**
|
||||
* 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) {
|
||||
any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user