Skip to content

Commit 441e8af

Browse files
atorralbaatorralba
committed
Decouple GrovyInjection.qll to reuse the taint tracking configuration
1 parent b08f417 commit 441e8af

File tree

5 files changed

+68
-75
lines changed

5 files changed

+68
-75
lines changed

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

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,9 @@
1111
*/
1212

1313
import java
14-
import semmle.code.java.dataflow.FlowSources
15-
import semmle.code.java.dataflow.TaintTracking
16-
import semmle.code.java.security.GroovyInjection
14+
import semmle.code.java.security.GroovyInjectionQuery
1715
import DataFlow::PathGraph
1816

19-
/**
20-
* A taint-tracking configuration for unsafe user input
21-
* that is used to evaluate a Groovy expression.
22-
*/
23-
class GroovyInjectionConfig extends TaintTracking::Configuration {
24-
GroovyInjectionConfig() { this = "GroovyInjectionConfig" }
25-
26-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
27-
28-
override predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink }
29-
30-
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
31-
any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
32-
}
33-
}
34-
3517
from DataFlow::PathNode source, DataFlow::PathNode sink, GroovyInjectionConfig conf
3618
where conf.hasFlowPath(source, sink)
3719
select sink.getNode(), source, sink, "Groovy Injection from $@.", source.getNode(),

‎java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ private module Frameworks {
9999
private import semmle.code.java.security.JexlInjectionSinkModels
100100
private import semmle.code.java.security.LdapInjection
101101
private import semmle.code.java.security.XPath
102-
private import semmle.code.java.security.GroovyInjection
102+
private import semmle.code.java.security.GroovyInjectionSinkModels
103103
private import semmle.code.java.frameworks.android.SQLite
104104
private import semmle.code.java.frameworks.Jdbc
105105
private import semmle.code.java.frameworks.SpringJdbc

‎java/ql/src/semmle/code/java/security/GroovyInjection.qll renamed to ‎java/ql/src/semmle/code/java/security/GroovyInjectionQuery.qll

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import java
44
import semmle.code.java.dataflow.DataFlow
55
import semmle.code.java.dataflow.ExternalFlow
6+
import semmle.code.java.dataflow.FlowSources
7+
import semmle.code.java.dataflow.TaintTracking
68
import semmle.code.java.frameworks.Networking
79

810
/** A data flow sink for Groovy expression injection vulnerabilities. */
@@ -25,47 +27,6 @@ private class DefaultGroovyInjectionSink extends GroovyInjectionSink {
2527
DefaultGroovyInjectionSink() { sinkNode(this, "groovy") }
2628
}
2729

28-
private class DefaultLdapInjectionSinkModel extends SinkModelCsv {
29-
override predicate row(string row) {
30-
row =
31-
[
32-
// Signatures are specified to exclude sinks of the type `File`
33-
"groovy.lang;GroovyShell;false;evaluate;(GroovyCodeSource);;Argument[0];groovy",
34-
"groovy.lang;GroovyShell;false;evaluate;(Reader);;Argument[0];groovy",
35-
"groovy.lang;GroovyShell;false;evaluate;(Reader,String);;Argument[0];groovy",
36-
"groovy.lang;GroovyShell;false;evaluate;(String);;Argument[0];groovy",
37-
"groovy.lang;GroovyShell;false;evaluate;(String,String);;Argument[0];groovy",
38-
"groovy.lang;GroovyShell;false;evaluate;(String,String,String);;Argument[0];groovy",
39-
"groovy.lang;GroovyShell;false;evaluate;(URI);;Argument[0];groovy",
40-
"groovy.lang;GroovyShell;false;parse;(Reader);;Argument[0];groovy",
41-
"groovy.lang;GroovyShell;false;parse;(Reader,String);;Argument[0];groovy",
42-
"groovy.lang;GroovyShell;false;parse;(String);;Argument[0];groovy",
43-
"groovy.lang;GroovyShell;false;parse;(String,String);;Argument[0];groovy",
44-
"groovy.lang;GroovyShell;false;parse;(URI);;Argument[0];groovy",
45-
"groovy.lang;GroovyShell;false;run;(GroovyCodeSource,String[]);;Argument[0];groovy",
46-
"groovy.lang;GroovyShell;false;run;(GroovyCodeSource,List);;Argument[0];groovy",
47-
"groovy.lang;GroovyShell;false;run;(Reader,String,String[]);;Argument[0];groovy",
48-
"groovy.lang;GroovyShell;false;run;(Reader,String,List);;Argument[0];groovy",
49-
"groovy.lang;GroovyShell;false;run;(String,String,String[]);;Argument[0];groovy",
50-
"groovy.lang;GroovyShell;false;run;(String,String,List);;Argument[0];groovy",
51-
"groovy.lang;GroovyShell;false;run;(URI,String[]);;Argument[0];groovy",
52-
"groovy.lang;GroovyShell;false;run;(URI,List);;Argument[0];groovy",
53-
"groovy.util;Eval;false;me;(String);;Argument[0];groovy",
54-
"groovy.util;Eval;false;me;(String,Object,String);;Argument[2];groovy",
55-
"groovy.util;Eval;false;x;(Object,String);;Argument[1];groovy",
56-
"groovy.util;Eval;false;xy;(Object,Object,String);;Argument[2];groovy",
57-
"groovy.util;Eval;false;xyz;(Object,Object,Object,String);;Argument[3];groovy",
58-
"groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource);;Argument[0];groovy",
59-
"groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource,boolean);;Argument[0];groovy",
60-
"groovy.lang;GroovyClassLoader;false;parseClass;(InputStream,String);;Argument[0];groovy",
61-
"groovy.lang;GroovyClassLoader;false;parseClass;(Reader,String);;Argument[0];groovy",
62-
"groovy.lang;GroovyClassLoader;false;parseClass;(String);;Argument[0];groovy",
63-
"groovy.lang;GroovyClassLoader;false;parseClass;(String,String);;Argument[0];groovy",
64-
"org.codehaus.groovy.control;CompilationUnit;false;compile;;;Argument[-1];groovy"
65-
]
66-
}
67-
}
68-
6930
/** A set of additional taint steps to consider when taint tracking Groovy related data flows. */
7031
private class DefaultGroovyInjectionAdditionalTaintStep extends GroovyInjectionAdditionalTaintStep {
7132
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
@@ -76,6 +37,22 @@ private class DefaultGroovyInjectionAdditionalTaintStep extends GroovyInjectionA
7637
}
7738
}
7839

40+
/**
41+
* A taint-tracking configuration for unsafe user input
42+
* that is used to evaluate a Groovy expression.
43+
*/
44+
class GroovyInjectionConfig extends TaintTracking::Configuration {
45+
GroovyInjectionConfig() { this = "GroovyInjectionConfig" }
46+
47+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
48+
49+
override predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink }
50+
51+
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
52+
any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
53+
}
54+
}
55+
7956
/**
8057
* Holds if `fromNode` to `toNode` is a dataflow step from a tainted string to
8158
* a `GroovyCodeSource` instance by calling `new GroovyCodeSource(tainted, ...)`.

‎java/ql/src/semmle/code/java/security/GroovyInjectionSinkModels.qll

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/** Provides sink models relating to Groovy injection vulnerabilities. */
2+
3+
private import semmle.code.java.dataflow.ExternalFlow
4+
5+
private class DefaultLdapInjectionSinkModel extends SinkModelCsv {
6+
override predicate row(string row) {
7+
row =
8+
[
9+
// Signatures are specified to exclude sinks of the type `File`
10+
"groovy.lang;GroovyShell;false;evaluate;(GroovyCodeSource);;Argument[0];groovy",
11+
"groovy.lang;GroovyShell;false;evaluate;(Reader);;Argument[0];groovy",
12+
"groovy.lang;GroovyShell;false;evaluate;(Reader,String);;Argument[0];groovy",
13+
"groovy.lang;GroovyShell;false;evaluate;(String);;Argument[0];groovy",
14+
"groovy.lang;GroovyShell;false;evaluate;(String,String);;Argument[0];groovy",
15+
"groovy.lang;GroovyShell;false;evaluate;(String,String,String);;Argument[0];groovy",
16+
"groovy.lang;GroovyShell;false;evaluate;(URI);;Argument[0];groovy",
17+
"groovy.lang;GroovyShell;false;parse;(Reader);;Argument[0];groovy",
18+
"groovy.lang;GroovyShell;false;parse;(Reader,String);;Argument[0];groovy",
19+
"groovy.lang;GroovyShell;false;parse;(String);;Argument[0];groovy",
20+
"groovy.lang;GroovyShell;false;parse;(String,String);;Argument[0];groovy",
21+
"groovy.lang;GroovyShell;false;parse;(URI);;Argument[0];groovy",
22+
"groovy.lang;GroovyShell;false;run;(GroovyCodeSource,String[]);;Argument[0];groovy",
23+
"groovy.lang;GroovyShell;false;run;(GroovyCodeSource,List);;Argument[0];groovy",
24+
"groovy.lang;GroovyShell;false;run;(Reader,String,String[]);;Argument[0];groovy",
25+
"groovy.lang;GroovyShell;false;run;(Reader,String,List);;Argument[0];groovy",
26+
"groovy.lang;GroovyShell;false;run;(String,String,String[]);;Argument[0];groovy",
27+
"groovy.lang;GroovyShell;false;run;(String,String,List);;Argument[0];groovy",
28+
"groovy.lang;GroovyShell;false;run;(URI,String[]);;Argument[0];groovy",
29+
"groovy.lang;GroovyShell;false;run;(URI,List);;Argument[0];groovy",
30+
"groovy.util;Eval;false;me;(String);;Argument[0];groovy",
31+
"groovy.util;Eval;false;me;(String,Object,String);;Argument[2];groovy",
32+
"groovy.util;Eval;false;x;(Object,String);;Argument[1];groovy",
33+
"groovy.util;Eval;false;xy;(Object,Object,String);;Argument[2];groovy",
34+
"groovy.util;Eval;false;xyz;(Object,Object,Object,String);;Argument[3];groovy",
35+
"groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource);;Argument[0];groovy",
36+
"groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource,boolean);;Argument[0];groovy",
37+
"groovy.lang;GroovyClassLoader;false;parseClass;(InputStream,String);;Argument[0];groovy",
38+
"groovy.lang;GroovyClassLoader;false;parseClass;(Reader,String);;Argument[0];groovy",
39+
"groovy.lang;GroovyClassLoader;false;parseClass;(String);;Argument[0];groovy",
40+
"groovy.lang;GroovyClassLoader;false;parseClass;(String,String);;Argument[0];groovy",
41+
"org.codehaus.groovy.control;CompilationUnit;false;compile;;;Argument[-1];groovy"
42+
]
43+
}
44+
}

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

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,19 @@
11
import java
22
import semmle.code.java.dataflow.TaintTracking
33
import semmle.code.java.dataflow.FlowSources
4-
import semmle.code.java.security.GroovyInjection
4+
import semmle.code.java.security.GroovyInjectionQuery
55
import TestUtilities.InlineExpectationsTest
66

7-
class Conf extends TaintTracking::Configuration {
8-
Conf() { this = "test:cwe:groovy-injection" }
9-
10-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
11-
12-
override predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink }
13-
14-
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
15-
any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
16-
}
17-
}
18-
197
class HasGroovyInjectionTest extends InlineExpectationsTest {
208
HasGroovyInjectionTest() { this = "HasGroovyInjectionTest" }
219

2210
override string getARelevantTag() { result = "hasGroovyInjection" }
2311

2412
override predicate hasActualResult(Location location, string element, string tag, string value) {
2513
tag = "hasGroovyInjection" and
26-
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
14+
exists(DataFlow::Node src, DataFlow::Node sink, GroovyInjectionConfig conf |
15+
conf.hasFlow(src, sink)
16+
|
2717
sink.getLocation() = location and
2818
element = sink.toString() and
2919
value = ""

0 commit comments

Comments
 (0)