Skip to content

Commit bc9942e

Browse files
aschackmullaschackmull
authored
Merge pull request github#12530 from aschackmull/java/refactor-dataflow-queries-3
Java: Refactor more dataflow queries to the new API (take 3)
2 parents a6e9d11 + 6408d7c commit bc9942e

18 files changed

+176
-68
lines changed

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,11 @@ private predicate isStartActivityOrServiceSink(DataFlow::Node arg) {
122122
}
123123

124124
/**
125+
* DEPRECATED: Use `SensitiveCommunicationFlow` instead.
126+
*
125127
* Taint configuration tracking flow from variables containing sensitive information to broadcast Intents.
126128
*/
127-
class SensitiveCommunicationConfig extends TaintTracking::Configuration {
129+
deprecated class SensitiveCommunicationConfig extends TaintTracking::Configuration {
128130
SensitiveCommunicationConfig() { this = "Sensitive Communication Configuration" }
129131

130132
override predicate isSource(DataFlow::Node source) {
@@ -148,3 +150,27 @@ class SensitiveCommunicationConfig extends TaintTracking::Configuration {
148150
this.isSink(node)
149151
}
150152
}
153+
154+
private module SensitiveCommunicationConfig implements DataFlow::ConfigSig {
155+
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveInfoExpr }
156+
157+
predicate isSink(DataFlow::Node sink) {
158+
isSensitiveBroadcastSink(sink)
159+
or
160+
isStartActivityOrServiceSink(sink)
161+
}
162+
163+
/**
164+
* Holds if broadcast doesn't specify receiving package name of the 3rd party app
165+
*/
166+
predicate isBarrier(DataFlow::Node node) { node instanceof ExplicitIntentSanitizer }
167+
168+
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
169+
isSink(node) and exists(c)
170+
}
171+
}
172+
173+
/**
174+
* Tracks taint flow from variables containing sensitive information to broadcast Intents.
175+
*/
176+
module SensitiveCommunicationFlow = TaintTracking::Make<SensitiveCommunicationConfig>;

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import semmle.code.java.dataflow.TaintTracking
66
import semmle.code.java.security.FragmentInjection
77

88
/**
9+
* DEPRECATED: Use `FragmentInjectionFlow` instead.
10+
*
911
* A taint-tracking configuration for unsafe user input
1012
* that is used to create Android fragments dynamically.
1113
*/
12-
class FragmentInjectionTaintConf extends TaintTracking::Configuration {
14+
deprecated class FragmentInjectionTaintConf extends TaintTracking::Configuration {
1315
FragmentInjectionTaintConf() { this = "FragmentInjectionTaintConf" }
1416

1517
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -20,3 +22,19 @@ class FragmentInjectionTaintConf extends TaintTracking::Configuration {
2022
any(FragmentInjectionAdditionalTaintStep c).step(n1, n2)
2123
}
2224
}
25+
26+
private module FragmentInjectionTaintConf implements DataFlow::ConfigSig {
27+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
28+
29+
predicate isSink(DataFlow::Node sink) { sink instanceof FragmentInjectionSink }
30+
31+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
32+
any(FragmentInjectionAdditionalTaintStep c).step(n1, n2)
33+
}
34+
}
35+
36+
/**
37+
* Taint-tracking flow for unsafe user input
38+
* that is used to create Android fragments dynamically.
39+
*/
40+
module FragmentInjectionTaintFlow = TaintTracking::Make<FragmentInjectionTaintConf>;

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ private import semmle.code.java.dataflow.DataFlow
99
private import IntentUriPermissionManipulation
1010

1111
/**
12+
* DEPRECATED: Use `IntentUriPermissionManipulationFlow` instead.
13+
*
1214
* A taint tracking configuration for user-provided Intents being returned to third party apps.
1315
*/
14-
class IntentUriPermissionManipulationConf extends TaintTracking::Configuration {
16+
deprecated class IntentUriPermissionManipulationConf extends TaintTracking::Configuration {
1517
IntentUriPermissionManipulationConf() { this = "UriPermissionManipulationConf" }
1618

1719
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -32,3 +34,23 @@ class IntentUriPermissionManipulationConf extends TaintTracking::Configuration {
3234
any(IntentUriPermissionManipulationAdditionalTaintStep c).step(node1, node2)
3335
}
3436
}
37+
38+
private module IntentUriPermissionManipulationConf implements DataFlow::ConfigSig {
39+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
40+
41+
predicate isSink(DataFlow::Node sink) { sink instanceof IntentUriPermissionManipulationSink }
42+
43+
predicate isBarrier(DataFlow::Node barrier) {
44+
barrier instanceof IntentUriPermissionManipulationSanitizer
45+
}
46+
47+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
48+
any(IntentUriPermissionManipulationAdditionalTaintStep c).step(node1, node2)
49+
}
50+
}
51+
52+
/**
53+
* Taint tracking flow for user-provided Intents being returned to third party apps.
54+
*/
55+
module IntentUriPermissionManipulationFlow =
56+
TaintTracking::Make<IntentUriPermissionManipulationConf>;

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import semmle.code.java.dataflow.FlowSources
55
import semmle.code.java.security.LogInjection
66

77
/**
8+
* DEPRECATED: Use `LogInjectionFlow` instead.
9+
*
810
* A taint-tracking configuration for tracking untrusted user input used in log entries.
911
*/
10-
class LogInjectionConfiguration extends TaintTracking::Configuration {
12+
deprecated class LogInjectionConfiguration extends TaintTracking::Configuration {
1113
LogInjectionConfiguration() { this = "LogInjectionConfiguration" }
1214

1315
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -20,3 +22,20 @@ class LogInjectionConfiguration extends TaintTracking::Configuration {
2022
any(LogInjectionAdditionalTaintStep c).step(node1, node2)
2123
}
2224
}
25+
26+
private module LogInjectionConfiguration implements DataFlow::ConfigSig {
27+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
28+
29+
predicate isSink(DataFlow::Node sink) { sink instanceof LogInjectionSink }
30+
31+
predicate isBarrier(DataFlow::Node node) { node instanceof LogInjectionSanitizer }
32+
33+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
34+
any(LogInjectionAdditionalTaintStep c).step(node1, node2)
35+
}
36+
}
37+
38+
/**
39+
* Taint-tracking flow for tracking untrusted user input used in log entries.
40+
*/
41+
module LogInjectionFlow = TaintTracking::Make<LogInjectionConfiguration>;

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@ import java
44
import Encryption
55
import semmle.code.java.dataflow.DataFlow
66

7-
/** A configuration for finding RSA ciphers initialized without using OAEP padding. */
8-
class RsaWithoutOaepConfig extends DataFlow::Configuration {
7+
/**
8+
* DEPRECATED: Use `RsaWithoutOaepFlow` instead.
9+
*
10+
* A configuration for finding RSA ciphers initialized without using OAEP padding.
11+
*/
12+
deprecated class RsaWithoutOaepConfig extends DataFlow::Configuration {
913
RsaWithoutOaepConfig() { this = "RsaWithoutOaepConfig" }
1014

1115
override predicate isSource(DataFlow::Node src) {
@@ -21,3 +25,21 @@ class RsaWithoutOaepConfig extends DataFlow::Configuration {
2125
exists(CryptoAlgoSpec cr | sink.asExpr() = cr.getAlgoSpec())
2226
}
2327
}
28+
29+
private module RsaWithoutOaepConfig implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node src) {
31+
exists(CompileTimeConstantExpr specExpr, string spec |
32+
specExpr.getStringValue() = spec and
33+
specExpr = src.asExpr() and
34+
spec.matches("RSA/%") and
35+
not spec.matches("%OAEP%")
36+
)
37+
}
38+
39+
predicate isSink(DataFlow::Node sink) {
40+
exists(CryptoAlgoSpec cr | sink.asExpr() = cr.getAlgoSpec())
41+
}
42+
}
43+
44+
/** Flow for finding RSA ciphers initialized without using OAEP padding. */
45+
module RsaWithoutOaepFlow = DataFlow::Make<RsaWithoutOaepConfig>;

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ import semmle.code.java.dataflow.FlowSources
55
import semmle.code.java.dataflow.TaintTracking
66
import semmle.code.java.security.UnsafeContentUriResolution
77

8-
/** A taint-tracking configuration to find paths from remote sources to content URI resolutions. */
9-
class UnsafeContentResolutionConf extends TaintTracking::Configuration {
8+
/**
9+
* DEPRECATED: Use `UnsafeContentUriResolutionFlow` instead.
10+
*
11+
* A taint-tracking configuration to find paths from remote sources to content URI resolutions.
12+
*/
13+
deprecated class UnsafeContentResolutionConf extends TaintTracking::Configuration {
1014
UnsafeContentResolutionConf() { this = "UnsafeContentResolutionConf" }
1115

1216
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
@@ -21,3 +25,20 @@ class UnsafeContentResolutionConf extends TaintTracking::Configuration {
2125
any(ContentUriResolutionAdditionalTaintStep s).step(node1, node2)
2226
}
2327
}
28+
29+
private module UnsafeContentResolutionConf implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
31+
32+
predicate isSink(DataFlow::Node sink) { sink instanceof ContentUriResolutionSink }
33+
34+
predicate isBarrier(DataFlow::Node sanitizer) {
35+
sanitizer instanceof ContentUriResolutionSanitizer
36+
}
37+
38+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
39+
any(ContentUriResolutionAdditionalTaintStep s).step(node1, node2)
40+
}
41+
}
42+
43+
/** Taint-tracking flow to find paths from remote sources to content URI resolutions. */
44+
module UnsafeContentResolutionFlow = TaintTracking::Make<UnsafeContentResolutionConf>;

‎java/ql/src/Security/CWE/CWE-117/LogInjection.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 semmle.code.java.security.LogInjectionQuery
16-
import DataFlow::PathGraph
16+
import LogInjectionFlow::PathGraph
1717

18-
from LogInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
19-
where cfg.hasFlowPath(source, sink)
18+
from LogInjectionFlow::PathNode source, LogInjectionFlow::PathNode sink
19+
where LogInjectionFlow::hasFlowPath(source, sink)
2020
select sink.getNode(), source, sink, "This log entry depends on a $@.", source.getNode(),
2121
"user-provided value"

‎java/ql/src/Security/CWE/CWE-266/IntentUriPermissionManipulation.ql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
import java
1616
import semmle.code.java.security.IntentUriPermissionManipulationQuery
1717
import semmle.code.java.dataflow.DataFlow
18-
import DataFlow::PathGraph
18+
import IntentUriPermissionManipulationFlow::PathGraph
1919

20-
from DataFlow::PathNode source, DataFlow::PathNode sink
21-
where any(IntentUriPermissionManipulationConf c).hasFlowPath(source, sink)
20+
from
21+
IntentUriPermissionManipulationFlow::PathNode source,
22+
IntentUriPermissionManipulationFlow::PathNode sink
23+
where IntentUriPermissionManipulationFlow::hasFlowPath(source, sink)
2224
select sink.getNode(), source, sink,
2325
"This Intent can be set with arbitrary flags from a $@, " +
2426
"and used to give access to internal content providers.", source.getNode(),

‎java/ql/src/Security/CWE/CWE-441/UnsafeContentUriResolution.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414

1515
import java
1616
import semmle.code.java.security.UnsafeContentUriResolutionQuery
17-
import DataFlow::PathGraph
17+
import UnsafeContentResolutionFlow::PathGraph
1818

19-
from DataFlow::PathNode src, DataFlow::PathNode sink
20-
where any(UnsafeContentResolutionConf c).hasFlowPath(src, sink)
19+
from UnsafeContentResolutionFlow::PathNode src, UnsafeContentResolutionFlow::PathNode sink
20+
where UnsafeContentResolutionFlow::hasFlowPath(src, sink)
2121
select sink.getNode(), src, sink,
2222
"This ContentResolver method that resolves a URI depends on a $@.", src.getNode(),
2323
"user-provided value"

‎java/ql/src/Security/CWE/CWE-470/FragmentInjection.ql

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

1414
import java
1515
import semmle.code.java.security.FragmentInjectionQuery
16-
import DataFlow::PathGraph
16+
import FragmentInjectionTaintFlow::PathGraph
1717

18-
from DataFlow::PathNode source, DataFlow::PathNode sink
19-
where any(FragmentInjectionTaintConf conf).hasFlowPath(source, sink)
18+
from FragmentInjectionTaintFlow::PathNode source, FragmentInjectionTaintFlow::PathNode sink
19+
where FragmentInjectionTaintFlow::hasFlowPath(source, sink)
2020
select sink.getNode(), source, sink,
2121
"Fragment depends on a $@, which may allow a malicious application to bypass access controls.",
2222
source.getNode(), "user-provided value"

0 commit comments

Comments
 (0)