Skip to content

Commit d309e15

Browse files
smowtonsmowton
authored
Merge pull request github#8748 from smowton/smowton/admin/dependent-dataflow-configs
Java: Avoid higher-numbered dataflow configs that depend on lower-numbered ones
2 parents 35471ff + 9050594 commit d309e15

File tree

8 files changed

+219
-10
lines changed

8 files changed

+219
-10
lines changed

‎config/identical-files.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
"csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll",
5252
"java/ql/lib/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
5353
"java/ql/lib/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
54+
"java/ql/lib/semmle/code/java/dataflow/internal/tainttracking3/TaintTrackingImpl.qll",
5455
"python/ql/lib/semmle/python/dataflow/new/internal/tainttracking1/TaintTrackingImpl.qll",
5556
"python/ql/lib/semmle/python/dataflow/new/internal/tainttracking2/TaintTrackingImpl.qll",
5657
"python/ql/lib/semmle/python/dataflow/new/internal/tainttracking3/TaintTrackingImpl.qll",
@@ -550,4 +551,4 @@
550551
"javascript/ql/lib/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll",
551552
"ruby/ql/lib/codeql/ruby/security/HttpToFileAccessCustomizations.qll"
552553
]
553-
}
554+
}

‎java/ql/lib/semmle/code/java/dataflow/TaintTracking3.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/**
2+
* Provides classes for performing local (intra-procedural) and
3+
* global (inter-procedural) taint-tracking analyses.
4+
*/
5+
module TaintTracking3 {
6+
import semmle.code.java.dataflow.internal.tainttracking3.TaintTrackingImpl
7+
}

‎java/ql/lib/semmle/code/java/dataflow/internal/tainttracking3/TaintTrackingImpl.qll

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/**
2+
* Provides an implementation of global (interprocedural) taint tracking.
3+
* This file re-exports the local (intraprocedural) taint-tracking analysis
4+
* from `TaintTrackingParameter::Public` and adds a global analysis, mainly
5+
* exposed through the `Configuration` class. For some languages, this file
6+
* exists in several identical copies, allowing queries to use multiple
7+
* `Configuration` classes that depend on each other without introducing
8+
* mutual recursion among those configurations.
9+
*/
10+
11+
import TaintTrackingParameter::Public
12+
private import TaintTrackingParameter::Private
13+
14+
/**
15+
* A configuration of interprocedural taint tracking analysis. This defines
16+
* sources, sinks, and any other configurable aspect of the analysis. Each
17+
* use of the taint tracking library must define its own unique extension of
18+
* this abstract class.
19+
*
20+
* A taint-tracking configuration is a special data flow configuration
21+
* (`DataFlow::Configuration`) that allows for flow through nodes that do not
22+
* necessarily preserve values but are still relevant from a taint tracking
23+
* perspective. (For example, string concatenation, where one of the operands
24+
* is tainted.)
25+
*
26+
* To create a configuration, extend this class with a subclass whose
27+
* characteristic predicate is a unique singleton string. For example, write
28+
*
29+
* ```ql
30+
* class MyAnalysisConfiguration extends TaintTracking::Configuration {
31+
* MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
32+
* // Override `isSource` and `isSink`.
33+
* // Optionally override `isSanitizer`.
34+
* // Optionally override `isSanitizerIn`.
35+
* // Optionally override `isSanitizerOut`.
36+
* // Optionally override `isSanitizerGuard`.
37+
* // Optionally override `isAdditionalTaintStep`.
38+
* }
39+
* ```
40+
*
41+
* Then, to query whether there is flow between some `source` and `sink`,
42+
* write
43+
*
44+
* ```ql
45+
* exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
46+
* ```
47+
*
48+
* Multiple configurations can coexist, but it is unsupported to depend on
49+
* another `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
50+
* overridden predicates that define sources, sinks, or additional steps.
51+
* Instead, the dependency should go to a `TaintTracking2::Configuration` or a
52+
* `DataFlow2::Configuration`, `DataFlow3::Configuration`, etc.
53+
*/
54+
abstract class Configuration extends DataFlow::Configuration {
55+
bindingset[this]
56+
Configuration() { any() }
57+
58+
/**
59+
* Holds if `source` is a relevant taint source.
60+
*
61+
* The smaller this predicate is, the faster `hasFlow()` will converge.
62+
*/
63+
// overridden to provide taint-tracking specific qldoc
64+
override predicate isSource(DataFlow::Node source) { none() }
65+
66+
/**
67+
* Holds if `source` is a relevant taint source with the given initial
68+
* `state`.
69+
*
70+
* The smaller this predicate is, the faster `hasFlow()` will converge.
71+
*/
72+
// overridden to provide taint-tracking specific qldoc
73+
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { none() }
74+
75+
/**
76+
* Holds if `sink` is a relevant taint sink
77+
*
78+
* The smaller this predicate is, the faster `hasFlow()` will converge.
79+
*/
80+
// overridden to provide taint-tracking specific qldoc
81+
override predicate isSink(DataFlow::Node sink) { none() }
82+
83+
/**
84+
* Holds if `sink` is a relevant taint sink accepting `state`.
85+
*
86+
* The smaller this predicate is, the faster `hasFlow()` will converge.
87+
*/
88+
// overridden to provide taint-tracking specific qldoc
89+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { none() }
90+
91+
/** Holds if the node `node` is a taint sanitizer. */
92+
predicate isSanitizer(DataFlow::Node node) { none() }
93+
94+
final override predicate isBarrier(DataFlow::Node node) {
95+
this.isSanitizer(node) or
96+
defaultTaintSanitizer(node)
97+
}
98+
99+
/**
100+
* Holds if the node `node` is a taint sanitizer when the flow state is
101+
* `state`.
102+
*/
103+
predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) { none() }
104+
105+
final override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
106+
this.isSanitizer(node, state)
107+
}
108+
109+
/** Holds if taint propagation into `node` is prohibited. */
110+
predicate isSanitizerIn(DataFlow::Node node) { none() }
111+
112+
/**
113+
* Holds if taint propagation into `node` is prohibited when the flow state is
114+
* `state`.
115+
*/
116+
predicate isSanitizerIn(DataFlow::Node node, DataFlow::FlowState state) { none() }
117+
118+
final override predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowState state) {
119+
this.isSanitizerIn(node, state)
120+
}
121+
122+
final override predicate isBarrierIn(DataFlow::Node node) { this.isSanitizerIn(node) }
123+
124+
/** Holds if taint propagation out of `node` is prohibited. */
125+
predicate isSanitizerOut(DataFlow::Node node) { none() }
126+
127+
final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
128+
129+
/**
130+
* Holds if taint propagation out of `node` is prohibited when the flow state is
131+
* `state`.
132+
*/
133+
predicate isSanitizerOut(DataFlow::Node node, DataFlow::FlowState state) { none() }
134+
135+
final override predicate isBarrierOut(DataFlow::Node node, DataFlow::FlowState state) {
136+
this.isSanitizerOut(node, state)
137+
}
138+
139+
/** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
140+
predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
141+
142+
final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
143+
this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
144+
}
145+
146+
/**
147+
* Holds if taint propagation through nodes guarded by `guard` is prohibited
148+
* when the flow state is `state`.
149+
*/
150+
predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
151+
152+
final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
153+
this.isSanitizerGuard(guard, state)
154+
}
155+
156+
/**
157+
* Holds if taint may propagate from `node1` to `node2` in addition to the normal data-flow and taint steps.
158+
*/
159+
predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
160+
161+
final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
162+
this.isAdditionalTaintStep(node1, node2) or
163+
defaultAdditionalTaintStep(node1, node2)
164+
}
165+
166+
/**
167+
* Holds if taint may propagate from `node1` to `node2` in addition to the normal data-flow and taint steps.
168+
* This step is only applicable in `state1` and updates the flow state to `state2`.
169+
*/
170+
predicate isAdditionalTaintStep(
171+
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
172+
DataFlow::FlowState state2
173+
) {
174+
none()
175+
}
176+
177+
final override predicate isAdditionalFlowStep(
178+
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
179+
DataFlow::FlowState state2
180+
) {
181+
this.isAdditionalTaintStep(node1, state1, node2, state2)
182+
}
183+
184+
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
185+
(this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
186+
defaultImplicitTaintRead(node, c)
187+
}
188+
189+
/**
190+
* Holds if taint may flow from `source` to `sink` for this configuration.
191+
*/
192+
// overridden to provide taint-tracking specific qldoc
193+
override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
194+
super.hasFlow(source, sink)
195+
}
196+
}

‎java/ql/lib/semmle/code/java/dataflow/internal/tainttracking3/TaintTrackingParameter.qll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import semmle.code.java.dataflow.internal.TaintTrackingUtil as Public
2+
3+
module Private {
4+
import semmle.code.java.dataflow.DataFlow3::DataFlow3 as DataFlow
5+
}

‎java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class Yaml extends RefType {
3030
Yaml() { this.getAnAncestor().hasQualifiedName("org.yaml.snakeyaml", "Yaml") }
3131
}
3232

33-
private class SafeYamlConstructionFlowConfig extends DataFlow2::Configuration {
33+
private class SafeYamlConstructionFlowConfig extends DataFlow3::Configuration {
3434
SafeYamlConstructionFlowConfig() { this = "SnakeYaml::SafeYamlConstructionFlowConfig" }
3535

3636
override predicate isSource(DataFlow::Node src) {
@@ -65,7 +65,7 @@ private class SnakeYamlParse extends MethodAccess {
6565
}
6666
}
6767

68-
private class SafeYamlFlowConfig extends DataFlow3::Configuration {
68+
private class SafeYamlFlowConfig extends DataFlow2::Configuration {
6969
SafeYamlFlowConfig() { this = "SnakeYaml::SafeYamlFlowConfig" }
7070

7171
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeYaml }

‎java/ql/lib/semmle/code/java/security/AndroidIntentRedirectionQuery.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
import java
44
import semmle.code.java.dataflow.FlowSources
5-
import semmle.code.java.dataflow.DataFlow3
5+
import semmle.code.java.dataflow.DataFlow2
66
import semmle.code.java.dataflow.TaintTracking
7-
import semmle.code.java.dataflow.TaintTracking2
7+
import semmle.code.java.dataflow.TaintTracking3
88
import semmle.code.java.security.AndroidIntentRedirection
99

1010
/**
@@ -38,7 +38,7 @@ private class OriginalIntentSanitizer extends IntentRedirectionSanitizer {
3838
* Data flow configuration used to discard incoming Intents
3939
* flowing directly to sinks that start Android components.
4040
*/
41-
private class SameIntentBeingRelaunchedConfiguration extends DataFlow3::Configuration {
41+
private class SameIntentBeingRelaunchedConfiguration extends DataFlow2::Configuration {
4242
SameIntentBeingRelaunchedConfiguration() { this = "SameIntentBeingRelaunchedConfiguration" }
4343

4444
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -74,7 +74,7 @@ private class IntentWithTaintedComponent extends DataFlow::Node {
7474
/**
7575
* A taint tracking configuration for tainted data flowing to an `Intent`'s component.
7676
*/
77-
private class TaintedIntentComponentConf extends TaintTracking2::Configuration {
77+
private class TaintedIntentComponentConf extends TaintTracking3::Configuration {
7878
TaintedIntentComponentConf() { this = "TaintedIntentComponentConf" }
7979

8080
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

‎java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import java
44
import semmle.code.java.frameworks.JAXB
55
import semmle.code.java.dataflow.DataFlow
6-
import semmle.code.java.dataflow.DataFlow2
76
import semmle.code.java.security.CleartextStorageQuery
87
import semmle.code.java.security.CleartextStoragePropertiesQuery
98

@@ -74,7 +73,7 @@ private Expr getInstanceInput(DataFlow::Node instance, RefType t) {
7473
)
7574
}
7675

77-
private class ClassStoreFlowConfig extends DataFlow2::Configuration {
76+
private class ClassStoreFlowConfig extends DataFlow::Configuration {
7877
ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" }
7978

8079
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }

‎java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java
44
private import semmle.code.java.dataflow.DataFlow4
55
private import semmle.code.java.dataflow.TaintTracking
6+
private import semmle.code.java.dataflow.TaintTracking2
67
private import semmle.code.java.security.SensitiveActions
78

89
/** A sink representing persistent storage that saves data in clear text. */
@@ -39,7 +40,7 @@ abstract class Storable extends Call {
3940
abstract Expr getAStore();
4041
}
4142

42-
private class SensitiveSourceFlowConfig extends TaintTracking::Configuration {
43+
private class SensitiveSourceFlowConfig extends TaintTracking2::Configuration {
4344
SensitiveSourceFlowConfig() { this = "SensitiveSourceFlowConfig" }
4445

4546
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }

0 commit comments

Comments
 (0)