Skip to content

Commit bb27ba7

Browse files
egregius313egregius313
authored
Merge pull request #12632 from egregius313/egregius313/java/android/refactor-android-query-libraries
Java: Refactor Android `Query.qll` libraries to new dataflow api
2 parents f1fe7af + 1bf4dd9 commit bb27ba7

14 files changed

+172
-96
lines changed

‎java/ql/lib/change-notes/2023-03-22-deprecate-webviewdubuggingenabledquery.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: deprecated
3+
---
4+
* The `WebViewDubuggingQuery` library has been renamed to `WebViewDebuggingQuery` to fix the typo in the file name. `WebViewDubuggingQuery` is now deprecated.

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,29 +106,29 @@ private class MissingPinningSink extends DataFlow::Node {
106106
}
107107

108108
/** Configuration for finding uses of non trusted URLs. */
109-
private class UntrustedUrlConfig extends TaintTracking::Configuration {
110-
UntrustedUrlConfig() { this = "UntrustedUrlConfig" }
111-
112-
override predicate isSource(DataFlow::Node node) {
109+
private module UntrustedUrlConfig implements DataFlow::ConfigSig {
110+
predicate isSource(DataFlow::Node node) {
113111
trustedDomain(_) and
114112
exists(string lit | lit = node.asExpr().(CompileTimeConstantExpr).getStringValue() |
115113
lit.matches("%://%") and // it's a URL
116114
not exists(string dom | trustedDomain(dom) and lit.matches("%" + dom + "%"))
117115
)
118116
}
119117

120-
override predicate isSink(DataFlow::Node node) { node instanceof MissingPinningSink }
118+
predicate isSink(DataFlow::Node node) { node instanceof MissingPinningSink }
121119
}
122120

121+
private module UntrustedUrlFlow = TaintTracking::Global<UntrustedUrlConfig>;
122+
123123
/** Holds if `node` is a network communication call for which certificate pinning is not implemented. */
124124
predicate missingPinning(DataFlow::Node node, string domain) {
125125
isAndroid() and
126126
node instanceof MissingPinningSink and
127127
(
128128
not trustedDomain(_) and domain = ""
129129
or
130-
exists(UntrustedUrlConfig conf, DataFlow::Node src |
131-
conf.hasFlow(src, node) and
130+
exists(DataFlow::Node src |
131+
UntrustedUrlFlow::flow(src, node) and
132132
domain = getDomain(src.asExpr())
133133
)
134134
)

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

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import semmle.code.java.dataflow.TaintTracking3
88
import semmle.code.java.security.AndroidIntentRedirection
99

1010
/**
11+
* DEPRECATED: Use `IntentRedirectionFlow` instead.
12+
*
1113
* A taint tracking configuration for tainted Intents being used to start Android components.
1214
*/
13-
class IntentRedirectionConfiguration extends TaintTracking::Configuration {
15+
deprecated class IntentRedirectionConfiguration extends TaintTracking::Configuration {
1416
IntentRedirectionConfiguration() { this = "IntentRedirectionConfiguration" }
1517

1618
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -26,31 +28,45 @@ class IntentRedirectionConfiguration extends TaintTracking::Configuration {
2628
}
2729
}
2830

31+
/** A taint tracking configuration for tainted Intents being used to start Android components. */
32+
module IntentRedirectionConfig implements DataFlow::ConfigSig {
33+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
34+
35+
predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }
36+
37+
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof IntentRedirectionSanitizer }
38+
39+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
40+
any(IntentRedirectionAdditionalTaintStep c).step(node1, node2)
41+
}
42+
}
43+
44+
/** Tracks the flow of tainted Intents being used to start Android components. */
45+
module IntentRedirectionFlow = TaintTracking::Global<IntentRedirectionConfig>;
46+
2947
/**
3048
* A sanitizer for sinks that receive the original incoming Intent,
3149
* since its component cannot be arbitrarily set.
3250
*/
3351
private class OriginalIntentSanitizer extends IntentRedirectionSanitizer {
34-
OriginalIntentSanitizer() { any(SameIntentBeingRelaunchedConfiguration c).hasFlowTo(this) }
52+
OriginalIntentSanitizer() { SameIntentBeingRelaunchedFlow::flowTo(this) }
3553
}
3654

3755
/**
3856
* Data flow configuration used to discard incoming Intents
3957
* flowing directly to sinks that start Android components.
4058
*/
41-
private class SameIntentBeingRelaunchedConfiguration extends DataFlow2::Configuration {
42-
SameIntentBeingRelaunchedConfiguration() { this = "SameIntentBeingRelaunchedConfiguration" }
59+
private module SameIntentBeingRelaunchedConfig implements DataFlow::ConfigSig {
60+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
4361

44-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
45-
46-
override predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }
62+
predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }
4763

48-
override predicate isBarrier(DataFlow::Node barrier) {
64+
predicate isBarrier(DataFlow::Node barrier) {
4965
// Don't discard the Intent if its original component is tainted
5066
barrier instanceof IntentWithTaintedComponent
5167
}
5268

53-
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
69+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
5470
// Intents being built with the copy constructor from the original Intent are discarded too
5571
exists(ClassInstanceExpr cie |
5672
cie.getConstructedType() instanceof TypeIntent and
@@ -61,29 +77,31 @@ private class SameIntentBeingRelaunchedConfiguration extends DataFlow2::Configur
6177
}
6278
}
6379

80+
private module SameIntentBeingRelaunchedFlow = DataFlow::Global<SameIntentBeingRelaunchedConfig>;
81+
6482
/** An `Intent` with a tainted component. */
6583
private class IntentWithTaintedComponent extends DataFlow::Node {
6684
IntentWithTaintedComponent() {
67-
exists(IntentSetComponent setExpr, TaintedIntentComponentConf conf |
85+
exists(IntentSetComponent setExpr |
6886
setExpr.getQualifier() = this.asExpr() and
69-
conf.hasFlowTo(DataFlow::exprNode(setExpr.getSink()))
87+
TaintedIntentComponentFlow::flowTo(DataFlow::exprNode(setExpr.getSink()))
7088
)
7189
}
7290
}
7391

7492
/**
7593
* A taint tracking configuration for tainted data flowing to an `Intent`'s component.
7694
*/
77-
private class TaintedIntentComponentConf extends TaintTracking3::Configuration {
78-
TaintedIntentComponentConf() { this = "TaintedIntentComponentConf" }
95+
private module TaintedIntentComponentConfig implements DataFlow::ConfigSig {
96+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
7997

80-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
81-
82-
override predicate isSink(DataFlow::Node sink) {
98+
predicate isSink(DataFlow::Node sink) {
8399
any(IntentSetComponent setComponent).getSink() = sink.asExpr()
84100
}
85101
}
86102

103+
private module TaintedIntentComponentFlow = TaintTracking::Global<TaintedIntentComponentConfig>;
104+
87105
/** A call to a method that changes the component of an `Intent`. */
88106
private class IntentSetComponent extends MethodAccess {
89107
int sinkArg;

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,25 @@ private class OnReceiveMethod extends Method {
1414
}
1515

1616
/** A configuration to detect whether the `action` of an `Intent` is checked. */
17-
private class VerifiedIntentConfig extends DataFlow::Configuration {
18-
VerifiedIntentConfig() { this = "VerifiedIntentConfig" }
19-
20-
override predicate isSource(DataFlow::Node src) {
17+
private module VerifiedIntentConfig implements DataFlow::ConfigSig {
18+
predicate isSource(DataFlow::Node src) {
2119
src.asParameter() = any(OnReceiveMethod orm).getIntentParameter()
2220
}
2321

24-
override predicate isSink(DataFlow::Node sink) {
22+
predicate isSink(DataFlow::Node sink) {
2523
exists(MethodAccess ma |
2624
ma.getMethod().hasQualifiedName("android.content", "Intent", "getAction") and
2725
sink.asExpr() = ma.getQualifier()
2826
)
2927
}
3028
}
3129

30+
private module VerifiedIntentFlow = DataFlow::Global<VerifiedIntentConfig>;
31+
3232
/** An `onReceive` method that doesn't verify the action of the intent it receives. */
3333
private class UnverifiedOnReceiveMethod extends OnReceiveMethod {
3434
UnverifiedOnReceiveMethod() {
35-
not any(VerifiedIntentConfig c).hasFlow(DataFlow::parameterNode(this.getIntentParameter()), _)
35+
not VerifiedIntentFlow::flow(DataFlow::parameterNode(this.getIntentParameter()), _)
3636
}
3737
}
3838

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,23 +91,23 @@ private predicate inputTypeFieldNotCached(Field f) {
9191
}
9292

9393
/** Configuration that finds uses of `setInputType` for non cached fields. */
94-
private class GoodInputTypeConf extends DataFlow::Configuration {
95-
GoodInputTypeConf() { this = "GoodInputTypeConf" }
96-
97-
override predicate isSource(DataFlow::Node node) {
94+
private module GoodInputTypeConfig implements DataFlow::ConfigSig {
95+
predicate isSource(DataFlow::Node node) {
9896
inputTypeFieldNotCached(node.asExpr().(FieldAccess).getField())
9997
}
10098

101-
override predicate isSink(DataFlow::Node node) { node.asExpr() = setInputTypeForId(_) }
99+
predicate isSink(DataFlow::Node node) { node.asExpr() = setInputTypeForId(_) }
102100

103-
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
101+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
104102
exists(OrBitwiseExpr bitOr |
105103
node1.asExpr() = bitOr.getAChildExpr() and
106104
node2.asExpr() = bitOr
107105
)
108106
}
109107
}
110108

109+
private module GoodInputTypeFlow = DataFlow::Global<GoodInputTypeConfig>;
110+
111111
/** Gets a regex indicating that an input field may contain sensitive data. */
112112
private string getInputSensitiveInfoRegex() {
113113
result =
@@ -130,8 +130,8 @@ AndroidEditableXmlElement getASensitiveCachedInput() {
130130
result.getId().regexpMatch(getInputSensitiveInfoRegex()) and
131131
(
132132
not inputTypeNotCached(result.getInputType()) and
133-
not exists(GoodInputTypeConf conf, DataFlow::Node sink |
134-
conf.hasFlowTo(sink) and
133+
not exists(DataFlow::Node sink |
134+
GoodInputTypeFlow::flowTo(sink) and
135135
sink.asExpr() = setInputTypeForId(result.getId())
136136
)
137137
)

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import semmle.code.java.security.RequestForgery
77
import semmle.code.java.security.UnsafeAndroidAccess
88

99
/**
10+
* DEPRECATED: Use `FetchUntrustedResourceFlow` instead.
11+
*
1012
* A taint configuration tracking flow from untrusted inputs to a resource fetching call.
1113
*/
12-
class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
14+
deprecated class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
1315
FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
1416

1517
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -20,3 +22,19 @@ class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
2022
sanitizer instanceof RequestForgerySanitizer
2123
}
2224
}
25+
26+
/**
27+
* A taint configuration tracking flow from untrusted inputs to a resource fetching call.
28+
*/
29+
module FetchUntrustedResourceConfig implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
31+
32+
predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
33+
34+
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof RequestForgerySanitizer }
35+
}
36+
37+
/**
38+
* Detects taint flow from untrusted inputs to a resource fetching call.
39+
*/
40+
module FetchUntrustedResourceFlow = TaintTracking::Global<FetchUntrustedResourceConfig>;

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/** Definitions for the Android Webview Debugging Enabled query */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.controlflow.Guards
6+
import semmle.code.java.security.SecurityTests
7+
8+
/** Holds if `ex` looks like a check that this is a debug build. */
9+
private predicate isDebugCheck(Expr ex) {
10+
exists(Expr subex, string debug |
11+
debug.toLowerCase().matches(["%debug%", "%test%"]) and
12+
subex.getParent*() = ex
13+
|
14+
subex.(VarAccess).getVariable().getName() = debug
15+
or
16+
subex.(MethodAccess).getMethod().hasName("getProperty") and
17+
subex.(MethodAccess).getAnArgument().(CompileTimeConstantExpr).getStringValue() = debug
18+
)
19+
}
20+
21+
/**
22+
* DEPRECATED: Use `WebviewDebugEnabledFlow` instead.
23+
*
24+
* A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values.
25+
*/
26+
deprecated class WebviewDebugEnabledConfig extends DataFlow::Configuration {
27+
WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }
28+
29+
override predicate isSource(DataFlow::Node node) {
30+
node.asExpr().(BooleanLiteral).getBooleanValue() = true
31+
}
32+
33+
override predicate isSink(DataFlow::Node node) {
34+
exists(MethodAccess ma |
35+
ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
36+
node.asExpr() = ma.getArgument(0)
37+
)
38+
}
39+
40+
override predicate isBarrier(DataFlow::Node node) {
41+
exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
42+
or
43+
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
44+
}
45+
}
46+
47+
/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
48+
module WebviewDebugEnabledConfig implements DataFlow::ConfigSig {
49+
predicate isSource(DataFlow::Node node) {
50+
node.asExpr().(BooleanLiteral).getBooleanValue() = true
51+
}
52+
53+
predicate isSink(DataFlow::Node node) {
54+
exists(MethodAccess ma |
55+
ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
56+
node.asExpr() = ma.getArgument(0)
57+
)
58+
}
59+
60+
predicate isBarrier(DataFlow::Node node) {
61+
exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
62+
or
63+
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
64+
}
65+
}
66+
67+
/**
68+
* Tracks instances of `setWebContentDebuggingEnabled` with `true` values.
69+
*/
70+
module WebviewDebugEnabledFlow = DataFlow::Global<WebviewDebugEnabledConfig>;

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

Lines changed: 8 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,11 @@
1-
/** Definitions for the Android Webview Debugging Enabled query */
1+
/**
2+
* DEPRECATED: Use `semmle.code.java.security.WebviewDebuggingEnabledQuery` instead.
3+
*
4+
* Definitions for the Android Webview Debugging Enabled query
5+
*/
26

37
import java
4-
import semmle.code.java.dataflow.DataFlow
5-
import semmle.code.java.controlflow.Guards
6-
import semmle.code.java.security.SecurityTests
8+
private import semmle.code.java.security.WebviewDebuggingEnabledQuery as WebviewDebuggingEnabledQuery
79

8-
/** Holds if `ex` looks like a check that this is a debug build. */
9-
private predicate isDebugCheck(Expr ex) {
10-
exists(Expr subex, string debug |
11-
debug.toLowerCase().matches(["%debug%", "%test%"]) and
12-
subex.getParent*() = ex
13-
|
14-
subex.(VarAccess).getVariable().getName() = debug
15-
or
16-
subex.(MethodAccess).getMethod().hasName("getProperty") and
17-
subex.(MethodAccess).getAnArgument().(CompileTimeConstantExpr).getStringValue() = debug
18-
)
19-
}
20-
21-
/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
22-
class WebviewDebugEnabledConfig extends DataFlow::Configuration {
23-
WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }
24-
25-
override predicate isSource(DataFlow::Node node) {
26-
node.asExpr().(BooleanLiteral).getBooleanValue() = true
27-
}
28-
29-
override predicate isSink(DataFlow::Node node) {
30-
exists(MethodAccess ma |
31-
ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
32-
node.asExpr() = ma.getArgument(0)
33-
)
34-
}
35-
36-
override predicate isBarrier(DataFlow::Node node) {
37-
exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
38-
or
39-
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
40-
}
41-
}
10+
deprecated class WebviewDebugEnabledConfig =
11+
WebviewDebuggingEnabledQuery::WebviewDebugEnabledConfig;

‎java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111
*/
1212

1313
import java
14-
import semmle.code.java.security.WebviewDubuggingEnabledQuery
15-
import DataFlow::PathGraph
14+
import semmle.code.java.security.WebviewDebuggingEnabledQuery
15+
import WebviewDebugEnabledFlow::PathGraph
1616

17-
from WebviewDebugEnabledConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink
18-
where conf.hasFlowPath(source, sink)
17+
from WebviewDebugEnabledFlow::PathNode source, WebviewDebugEnabledFlow::PathNode sink
18+
where WebviewDebugEnabledFlow::flowPath(source, sink)
1919
select sink, source, sink, "Webview debugging is enabled."

0 commit comments

Comments
 (0)