Skip to content

Commit aeff6d3

Browse files
authored
Merge pull request github#12808 from egregius313/egregius313/java/dataflow/refactor-experimental
Java: Refactor experimental queries to new DataFlow API
2 parents 52bc43b + 7d0680a commit aeff6d3

File tree

53 files changed

+503
-519
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+503
-519
lines changed

java/ql/src/experimental/Security/CWE/CWE-020/Log4jJndiInjection.ql

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616
*/
1717

1818
import java
19+
import semmle.code.java.dataflow.TaintTracking
1920
import semmle.code.java.dataflow.FlowSources
2021
import semmle.code.java.dataflow.ExternalFlow
21-
import DataFlow::PathGraph
22+
import Log4jInjectionFlow::PathGraph
2223

2324
private class ActivateModels extends ActiveExperimentalModels {
2425
ActivateModels() { this = "log4j-injection" }
@@ -41,17 +42,20 @@ class Log4jInjectionSanitizer extends DataFlow::Node {
4142
/**
4243
* A taint-tracking configuration for tracking untrusted user input used in log entries.
4344
*/
44-
class Log4jInjectionConfiguration extends TaintTracking::Configuration {
45-
Log4jInjectionConfiguration() { this = "Log4jInjectionConfiguration" }
45+
module Log4jInjectionConfig implements DataFlow::ConfigSig {
46+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
4647

47-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
48+
predicate isSink(DataFlow::Node sink) { sink instanceof Log4jInjectionSink }
4849

49-
override predicate isSink(DataFlow::Node sink) { sink instanceof Log4jInjectionSink }
50-
51-
override predicate isSanitizer(DataFlow::Node node) { node instanceof Log4jInjectionSanitizer }
50+
predicate isBarrier(DataFlow::Node node) { node instanceof Log4jInjectionSanitizer }
5251
}
5352

54-
from Log4jInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
55-
where cfg.hasFlowPath(source, sink)
53+
/**
54+
* Taint-tracking flow for tracking untrusted user input used in log entries.
55+
*/
56+
module Log4jInjectionFlow = TaintTracking::Global<Log4jInjectionConfig>;
57+
58+
from Log4jInjectionFlow::PathNode source, Log4jInjectionFlow::PathNode sink
59+
where Log4jInjectionFlow::flowPath(source, sink)
5660
select sink.getNode(), source, sink, "Log4j log entry depends on a $@.", source.getNode(),
5761
"user-provided value"

java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.ql

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import java
1515
import semmle.code.java.dataflow.TaintTracking
1616
import semmle.code.java.dataflow.FlowSources
1717
import semmle.code.java.dataflow.ExternalFlow
18-
import DataFlow::PathGraph
18+
import RemoteUrlToOpenStreamFlow::PathGraph
1919

2020
class UrlConstructor extends ClassInstanceExpr {
2121
UrlConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl }
@@ -28,30 +28,32 @@ class UrlConstructor extends ClassInstanceExpr {
2828
}
2929
}
3030

31-
class RemoteUrlToOpenStreamFlowConfig extends TaintTracking::Configuration {
32-
RemoteUrlToOpenStreamFlowConfig() { this = "OpenStream::RemoteURLToOpenStreamFlowConfig" }
31+
module RemoteUrlToOpenStreamFlowConfig implements DataFlow::ConfigSig {
32+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
3333

34-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
35-
36-
override predicate isSink(DataFlow::Node sink) {
34+
predicate isSink(DataFlow::Node sink) {
3735
exists(MethodAccess m |
3836
sink.asExpr() = m.getQualifier() and m.getMethod() instanceof UrlOpenStreamMethod
3937
)
4038
or
4139
sinkNode(sink, "url-open-stream")
4240
}
4341

44-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
42+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
4543
exists(UrlConstructor u |
4644
node1.asExpr() = u.stringArg() and
4745
node2.asExpr() = u
4846
)
4947
}
5048
}
5149

52-
from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess call
50+
module RemoteUrlToOpenStreamFlow = TaintTracking::Global<RemoteUrlToOpenStreamFlowConfig>;
51+
52+
from
53+
RemoteUrlToOpenStreamFlow::PathNode source, RemoteUrlToOpenStreamFlow::PathNode sink,
54+
MethodAccess call
5355
where
5456
sink.getNode().asExpr() = call.getQualifier() and
55-
any(RemoteUrlToOpenStreamFlowConfig c).hasFlowPath(source, sink)
57+
RemoteUrlToOpenStreamFlow::flowPath(source, sink)
5658
select call, source, sink,
5759
"URL on which openStream is called may have been constructed from remote source."

java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@
1313
*/
1414

1515
import java
16+
import semmle.code.java.dataflow.TaintTracking
1617
import semmle.code.java.dataflow.ExternalFlow
1718
import semmle.code.java.dataflow.FlowSources
1819
import semmle.code.java.security.PathCreation
1920
import JFinalController
2021
import semmle.code.java.security.PathSanitizer
21-
import DataFlow::PathGraph
22+
import InjectFilePathFlow::PathGraph
2223

2324
private class ActivateModels extends ActiveExperimentalModels {
2425
ActivateModels() { this = "file-path-injection" }
@@ -47,24 +48,24 @@ class NormalizedPathNode extends DataFlow::Node {
4748
}
4849
}
4950

50-
class InjectFilePathConfig extends TaintTracking::Configuration {
51-
InjectFilePathConfig() { this = "InjectFilePathConfig" }
51+
module InjectFilePathConfig implements DataFlow::ConfigSig {
52+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
5253

53-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
54-
55-
override predicate isSink(DataFlow::Node sink) {
54+
predicate isSink(DataFlow::Node sink) {
5655
sink.asExpr() = any(PathCreation p).getAnInput() and
5756
not sink instanceof NormalizedPathNode
5857
}
5958

60-
override predicate isSanitizer(DataFlow::Node node) {
59+
predicate isBarrier(DataFlow::Node node) {
6160
exists(Type t | t = node.getType() | t instanceof BoxedType or t instanceof PrimitiveType)
6261
or
6362
node instanceof PathInjectionSanitizer
6463
}
6564
}
6665

67-
from DataFlow::PathNode source, DataFlow::PathNode sink, InjectFilePathConfig conf
68-
where conf.hasFlowPath(source, sink)
66+
module InjectFilePathFlow = TaintTracking::Global<InjectFilePathConfig>;
67+
68+
from InjectFilePathFlow::PathNode source, InjectFilePathFlow::PathNode sink
69+
where InjectFilePathFlow::flowPath(source, sink)
6970
select sink.getNode(), source, sink, "External control of file name or path due to $@.",
7071
source.getNode(), "user-provided value"

java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.ql

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,24 @@
1313
*/
1414

1515
import java
16-
import DataFlow::PathGraph
1716
import MyBatisCommonLib
1817
import MyBatisAnnotationSqlInjectionLib
1918
import semmle.code.java.dataflow.FlowSources
19+
import semmle.code.java.dataflow.TaintTracking
20+
import MyBatisAnnotationSqlInjectionFlow::PathGraph
2021

21-
private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::Configuration {
22-
MyBatisAnnotationSqlInjectionConfiguration() { this = "MyBatis annotation sql injection" }
22+
private module MyBatisAnnotationSqlInjectionConfig implements DataFlow::ConfigSig {
23+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2324

24-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
25+
predicate isSink(DataFlow::Node sink) { sink instanceof MyBatisAnnotatedMethodCallArgument }
2526

26-
override predicate isSink(DataFlow::Node sink) {
27-
sink instanceof MyBatisAnnotatedMethodCallArgument
28-
}
29-
30-
override predicate isSanitizer(DataFlow::Node node) {
27+
predicate isBarrier(DataFlow::Node node) {
3128
node.getType() instanceof PrimitiveType or
3229
node.getType() instanceof BoxedType or
3330
node.getType() instanceof NumberType
3431
}
3532

36-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
33+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
3734
exists(MethodAccess ma |
3835
ma.getMethod().getDeclaringType() instanceof TypeObject and
3936
ma.getMethod().getName() = "toString" and
@@ -43,12 +40,15 @@ private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::
4340
}
4441
}
4542

43+
private module MyBatisAnnotationSqlInjectionFlow =
44+
TaintTracking::Global<MyBatisAnnotationSqlInjectionConfig>;
45+
4646
from
47-
MyBatisAnnotationSqlInjectionConfiguration cfg, DataFlow::PathNode source,
48-
DataFlow::PathNode sink, IbatisSqlOperationAnnotation isoa, MethodAccess ma,
49-
string unsafeExpression
47+
MyBatisAnnotationSqlInjectionFlow::PathNode source,
48+
MyBatisAnnotationSqlInjectionFlow::PathNode sink, IbatisSqlOperationAnnotation isoa,
49+
MethodAccess ma, string unsafeExpression
5050
where
51-
cfg.hasFlowPath(source, sink) and
51+
MyBatisAnnotationSqlInjectionFlow::flowPath(source, sink) and
5252
ma.getAnArgument() = sink.getNode().asExpr() and
5353
myBatisSqlOperationAnnotationFromMethod(ma.getMethod(), isoa) and
5454
unsafeExpression = getAMybatisAnnotationSqlValue(isoa) and

java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,23 @@ private predicate propertiesKey(DataFlow::Node prop, string key) {
1717
}
1818

1919
/** A data flow configuration tracing flow from ibatis `Configuration.getVariables()` to a store into a `Properties` object. */
20-
private class PropertiesFlowConfig extends DataFlow2::Configuration {
21-
PropertiesFlowConfig() { this = "PropertiesFlowConfig" }
22-
23-
override predicate isSource(DataFlow::Node src) {
20+
private module PropertiesFlowConfig implements DataFlow::ConfigSig {
21+
predicate isSource(DataFlow::Node src) {
2422
exists(MethodAccess ma | ma.getMethod() instanceof IbatisConfigurationGetVariablesMethod |
2523
src.asExpr() = ma
2624
)
2725
}
2826

29-
override predicate isSink(DataFlow::Node sink) { propertiesKey(sink, _) }
27+
predicate isSink(DataFlow::Node sink) { propertiesKey(sink, _) }
3028
}
3129

30+
private module PropertiesFlow = DataFlow::Global<PropertiesFlowConfig>;
31+
3232
/** Gets a `Properties` key that may map onto a Mybatis `Configuration` variable. */
3333
string getAMybatisConfigurationVariableKey() {
34-
exists(PropertiesFlowConfig conf, DataFlow::Node n |
34+
exists(DataFlow::Node n |
3535
propertiesKey(n, result) and
36-
conf.hasFlowTo(n)
36+
PropertiesFlow::flowTo(n)
3737
)
3838
}
3939

java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,24 @@
1313
*/
1414

1515
import java
16-
import DataFlow::PathGraph
1716
import MyBatisCommonLib
1817
import MyBatisMapperXmlSqlInjectionLib
1918
import semmle.code.xml.MyBatisMapperXML
2019
import semmle.code.java.dataflow.FlowSources
20+
import MyBatisMapperXmlSqlInjectionFlow::PathGraph
2121

22-
private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::Configuration {
23-
MyBatisMapperXmlSqlInjectionConfiguration() { this = "MyBatis mapper xml sql injection" }
22+
private module MyBatisMapperXmlSqlInjectionConfig implements DataFlow::ConfigSig {
23+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2424

25-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
25+
predicate isSink(DataFlow::Node sink) { sink instanceof MyBatisMapperMethodCallAnArgument }
2626

27-
override predicate isSink(DataFlow::Node sink) {
28-
sink instanceof MyBatisMapperMethodCallAnArgument
29-
}
30-
31-
override predicate isSanitizer(DataFlow::Node node) {
27+
predicate isBarrier(DataFlow::Node node) {
3228
node.getType() instanceof PrimitiveType or
3329
node.getType() instanceof BoxedType or
3430
node.getType() instanceof NumberType
3531
}
3632

37-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
33+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
3834
exists(MethodAccess ma |
3935
ma.getMethod().getDeclaringType() instanceof TypeObject and
4036
ma.getMethod().getName() = "toString" and
@@ -44,11 +40,15 @@ private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::C
4440
}
4541
}
4642

43+
private module MyBatisMapperXmlSqlInjectionFlow =
44+
TaintTracking::Global<MyBatisMapperXmlSqlInjectionConfig>;
45+
4746
from
48-
MyBatisMapperXmlSqlInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
49-
MyBatisMapperXmlElement mmxe, MethodAccess ma, string unsafeExpression
47+
MyBatisMapperXmlSqlInjectionFlow::PathNode source,
48+
MyBatisMapperXmlSqlInjectionFlow::PathNode sink, MyBatisMapperXmlElement mmxe, MethodAccess ma,
49+
string unsafeExpression
5050
where
51-
cfg.hasFlowPath(source, sink) and
51+
MyBatisMapperXmlSqlInjectionFlow::flowPath(source, sink) and
5252
ma.getAnArgument() = sink.getNode().asExpr() and
5353
myBatisMapperXmlElementFromMethod(ma.getMethod(), mmxe) and
5454
unsafeExpression = getAMybatisXmlSetValue(mmxe) and

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
import java
1515
import BeanShellInjection
1616
import semmle.code.java.dataflow.FlowSources
17-
import DataFlow::PathGraph
17+
import semmle.code.java.dataflow.TaintTracking
18+
import BeanShellInjectionFlow::PathGraph
1819

19-
class BeanShellInjectionConfig extends TaintTracking::Configuration {
20-
BeanShellInjectionConfig() { this = "BeanShellInjectionConfig" }
20+
module BeanShellInjectionConfig implements DataFlow::ConfigSig {
21+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2122

22-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
23+
predicate isSink(DataFlow::Node sink) { sink instanceof BeanShellInjectionSink }
2324

24-
override predicate isSink(DataFlow::Node sink) { sink instanceof BeanShellInjectionSink }
25-
26-
override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) {
25+
predicate isAdditionalFlowStep(DataFlow::Node prod, DataFlow::Node succ) {
2726
exists(ClassInstanceExpr cie |
2827
cie.getConstructedType()
2928
.hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and
@@ -42,7 +41,9 @@ class BeanShellInjectionConfig extends TaintTracking::Configuration {
4241
}
4342
}
4443

45-
from DataFlow::PathNode source, DataFlow::PathNode sink, BeanShellInjectionConfig conf
46-
where conf.hasFlowPath(source, sink)
44+
module BeanShellInjectionFlow = TaintTracking::Global<BeanShellInjectionConfig>;
45+
46+
from BeanShellInjectionFlow::PathNode source, BeanShellInjectionFlow::PathNode sink
47+
where BeanShellInjectionFlow::flowPath(source, sink)
4748
select sink.getNode(), source, sink, "BeanShell injection from $@.", source.getNode(),
4849
"this user input"

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313

1414
import java
1515
import InsecureDexLoading
16-
import DataFlow::PathGraph
16+
import InsecureDexFlow::PathGraph
1717

18-
from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureDexConfiguration conf
19-
where conf.hasFlowPath(source, sink)
18+
from InsecureDexFlow::PathNode source, InsecureDexFlow::PathNode sink
19+
where InsecureDexFlow::flowPath(source, sink)
2020
select sink.getNode(), source, sink, "Potential arbitrary code execution due to $@.",
2121
source.getNode(), "a value loaded from a world-writable source."

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
11
import java
2+
import semmle.code.java.dataflow.TaintTracking
23
import semmle.code.java.dataflow.FlowSources
34

45
/**
56
* A taint-tracking configuration detecting unsafe use of a
67
* `DexClassLoader` by an Android app.
78
*/
8-
class InsecureDexConfiguration extends TaintTracking::Configuration {
9-
InsecureDexConfiguration() { this = "Insecure Dex File Load" }
9+
module InsecureDexConfig implements DataFlow::ConfigSig {
10+
predicate isSource(DataFlow::Node source) { source instanceof InsecureDexSource }
1011

11-
override predicate isSource(DataFlow::Node source) { source instanceof InsecureDexSource }
12+
predicate isSink(DataFlow::Node sink) { sink instanceof InsecureDexSink }
1213

13-
override predicate isSink(DataFlow::Node sink) { sink instanceof InsecureDexSink }
14-
15-
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
16-
flowStep(pred, succ)
17-
}
14+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { flowStep(pred, succ) }
1815
}
1916

17+
module InsecureDexFlow = TaintTracking::Global<InsecureDexConfig>;
18+
2019
/** A data flow source for insecure Dex class loading vulnerabilities. */
2120
abstract class InsecureDexSource extends DataFlow::Node { }
2221

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
import java
1515
import JShellInjection
1616
import semmle.code.java.dataflow.FlowSources
17-
import DataFlow::PathGraph
17+
import semmle.code.java.dataflow.TaintTracking
18+
import JShellInjectionFlow::PathGraph
1819

19-
class JShellInjectionConfiguration extends TaintTracking::Configuration {
20-
JShellInjectionConfiguration() { this = "JShellInjectionConfiguration" }
20+
module JShellInjectionConfig implements DataFlow::ConfigSig {
21+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2122

22-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
23+
predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink }
2324

24-
override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink }
25-
26-
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
25+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
2726
exists(SourceCodeAnalysisAnalyzeCompletionCall scaacc |
2827
scaacc.getArgument(0) = pred.asExpr() and scaacc = succ.asExpr()
2928
)
@@ -34,7 +33,9 @@ class JShellInjectionConfiguration extends TaintTracking::Configuration {
3433
}
3534
}
3635

37-
from DataFlow::PathNode source, DataFlow::PathNode sink, JShellInjectionConfiguration conf
38-
where conf.hasFlowPath(source, sink)
36+
module JShellInjectionFlow = TaintTracking::Global<JShellInjectionConfig>;
37+
38+
from JShellInjectionFlow::PathNode source, JShellInjectionFlow::PathNode sink
39+
where JShellInjectionFlow::flowPath(source, sink)
3940
select sink.getNode(), source, sink, "JShell injection from $@.", source.getNode(),
4041
"this user input"

0 commit comments

Comments
 (0)