Skip to content

Commit 02b440b

Browse files
Merge pull request #6599 from joefarebrother/android-sensitive-communication
Java: Promote android sensitive broadcast query
2 parents e5e1046 + ba95d46 commit 02b440b

File tree

13 files changed

+238
-241
lines changed

13 files changed

+238
-241
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* The query "Leaking sensitive information through an implicit Intent" (`java/android/sensitive-communication`) has been promoted from experimental to the main query pack. Its results will now appear by default. The query was originally [submitted as an experimental query by @luchua-bc.](https://github.com/github/codeql/pull/4512)

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ private module Frameworks {
7979
private import internal.ContainerFlow
8080
private import semmle.code.java.frameworks.android.Android
8181
private import semmle.code.java.frameworks.android.Intent
82+
private import semmle.code.java.frameworks.android.SQLite
8283
private import semmle.code.java.frameworks.android.XssSinks
8384
private import semmle.code.java.frameworks.ApacheHttp
8485
private import semmle.code.java.frameworks.apache.Collections
@@ -114,8 +115,6 @@ private module Frameworks {
114115
private import semmle.code.java.security.OgnlInjection
115116
private import semmle.code.java.security.XPath
116117
private import semmle.code.java.security.XsltInjection
117-
private import semmle.code.java.frameworks.android.Android
118-
private import semmle.code.java.frameworks.android.SQLite
119118
private import semmle.code.java.frameworks.Jdbc
120119
private import semmle.code.java.frameworks.SpringJdbc
121120
private import semmle.code.java.frameworks.MyBatis
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/** Provides definitions to reason about Android Sensitive Communication queries */
2+
3+
import java
4+
import semmle.code.java.dataflow.TaintTracking
5+
import semmle.code.java.frameworks.android.Intent
6+
import semmle.code.java.security.SensitiveActions
7+
8+
/**
9+
* Gets regular expression for matching names of Android variables that indicate the value being held contains sensitive information.
10+
*/
11+
private string getAndroidSensitiveInfoRegex() { result = "(?i).*(email|phone|ticket).*" }
12+
13+
/** Finds variables that hold sensitive information judging by their names. */
14+
private class SensitiveInfoExpr extends Expr {
15+
SensitiveInfoExpr() {
16+
exists(Variable v | this = v.getAnAccess() |
17+
v.getName().regexpMatch([getCommonSensitiveInfoRegex(), getAndroidSensitiveInfoRegex()])
18+
)
19+
}
20+
}
21+
22+
private predicate maybeNullArg(Expr ex) {
23+
exists(DataFlow::Node src, DataFlow::Node sink, MethodAccess ma |
24+
ex = ma.getAnArgument() and
25+
sink.asExpr() = ex and
26+
src.asExpr() instanceof NullLiteral
27+
|
28+
DataFlow::localFlow(src, sink)
29+
)
30+
}
31+
32+
private predicate maybeEmptyArrayArg(Expr ex) {
33+
exists(DataFlow::Node src, DataFlow::Node sink, MethodAccess ma |
34+
ex = ma.getAnArgument() and
35+
sink.asExpr() = ex and
36+
src.asExpr().(ArrayCreationExpr).getFirstDimensionSize() = 0
37+
|
38+
DataFlow::localFlow(src, sink)
39+
)
40+
}
41+
42+
/**
43+
* Holds if a `sendBroadcast` call doesn't specify receiver permission.
44+
*/
45+
private predicate isSensitiveBroadcastSink(DataFlow::Node sendBroadcastCallArg) {
46+
exists(MethodAccess ma, string name | ma.getMethod().hasName(name) |
47+
ma.getMethod().getDeclaringType().getASourceSupertype*() instanceof TypeContext and
48+
sendBroadcastCallArg.asExpr() = ma.getAnArgument() and
49+
(
50+
name = "sendBroadcast" and
51+
(
52+
// sendBroadcast(Intent intent)
53+
ma.getNumArgument() = 1
54+
or
55+
// sendBroadcast(Intent intent, String receiverPermission)
56+
maybeNullArg(ma.getArgument(1))
57+
)
58+
or
59+
name = "sendBroadcastAsUser" and
60+
(
61+
// sendBroadcastAsUser(Intent intent, UserHandle user)
62+
ma.getNumArgument() = 2
63+
or
64+
// sendBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission)
65+
maybeNullArg(ma.getArgument(2))
66+
)
67+
or
68+
// sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions)
69+
name = "sendBroadcastWithMultiplePermissions" and
70+
maybeEmptyArrayArg(ma.getArgument(1))
71+
or
72+
// Method calls of `sendOrderedBroadcast` whose second argument is always `receiverPermission`
73+
name = "sendOrderedBroadcast" and
74+
(
75+
// sendOrderedBroadcast(Intent intent, String receiverPermission)
76+
// sendOrderedBroadcast(Intent intent, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
77+
maybeNullArg(ma.getArgument(1)) and
78+
ma.getNumArgument() = [2, 7]
79+
or
80+
// sendOrderedBroadcast(Intent intent, String receiverPermission, String receiverAppOp, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
81+
maybeNullArg(ma.getArgument(1)) and
82+
maybeNullArg(ma.getArgument(2)) and
83+
ma.getNumArgument() = 8
84+
)
85+
or
86+
// sendOrderedBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
87+
name = "sendOrderedBroadcastAsUser" and
88+
maybeNullArg(ma.getArgument(2))
89+
or
90+
// sendStickyBroadcast(Intent intent)
91+
// sendStickyBroadcast(Intent intent, Bundle options)
92+
// sendStickyBroadcastAsUser(Intent intent, UserHandle user)
93+
// sendStickyOrderedBroadcast(Intent intent, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
94+
// sendStickyOrderedBroadcastAsUser(Intent intent, UserHandle user, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
95+
name =
96+
[
97+
"sendStickyBroadcast", "sendStickyBroadcastAsUser", "sendStickyOrderedBroadcast",
98+
"sendStickyOrderedBroadcastAsUser"
99+
]
100+
)
101+
)
102+
}
103+
104+
/**
105+
* Holds if `arg` is an argument in a use of a `startActivity` or `startService` method that sends an Intent to another application.
106+
*/
107+
private predicate isStartActivityOrServiceSink(DataFlow::Node arg) {
108+
exists(MethodAccess ma, string name | ma.getMethod().hasName(name) |
109+
arg.asExpr() = ma.getArgument(0) and
110+
ma.getMethod().getDeclaringType().getASourceSupertype*() instanceof TypeContext and
111+
// startActivity(Intent intent)
112+
// startActivity(Intent intent, Bundle options)
113+
// startActivities(Intent[] intents)
114+
// startActivities(Intent[] intents, Bundle options)
115+
// startService(Intent service)
116+
// startForegroundService(Intent service)
117+
// bindService (Intent service, int flags, Executor executor, ServiceConnection conn)
118+
// bindService (Intent service, Executor executor, ServiceConnection conn)
119+
name =
120+
["startActivity", "startActivities", "startService", "startForegroundService", "bindService"]
121+
)
122+
}
123+
124+
private predicate isCleanIntent(Expr intent) {
125+
intent.getType() instanceof TypeIntent and
126+
(
127+
exists(MethodAccess setRecieverMa |
128+
setRecieverMa.getQualifier() = intent and
129+
setRecieverMa.getMethod().hasName(["setPackage", "setClass", "setClassName", "setComponent"])
130+
)
131+
or
132+
// Handle the cases where the PackageContext and Class are set at construction time
133+
// Intent(Context packageContext, Class<?> cls)
134+
// Intent(String action, Uri uri, Context packageContext, Class<?> cls)
135+
exists(ConstructorCall cc | cc = intent |
136+
cc.getConstructedType() instanceof TypeIntent and
137+
cc.getNumArgument() > 1 and
138+
(
139+
cc.getArgument(0).getType() instanceof TypeContext and
140+
not maybeNullArg(cc.getArgument(1))
141+
or
142+
cc.getArgument(2).getType() instanceof TypeContext and
143+
not maybeNullArg(cc.getArgument(3))
144+
)
145+
)
146+
)
147+
}
148+
149+
/**
150+
* Taint configuration tracking flow from variables containing sensitive information to broadcast Intents.
151+
*/
152+
class SensitiveCommunicationConfig extends TaintTracking::Configuration {
153+
SensitiveCommunicationConfig() { this = "Sensitive Communication Configuration" }
154+
155+
override predicate isSource(DataFlow::Node source) {
156+
source.asExpr() instanceof SensitiveInfoExpr
157+
}
158+
159+
override predicate isSink(DataFlow::Node sink) {
160+
isSensitiveBroadcastSink(sink)
161+
or
162+
isStartActivityOrServiceSink(sink)
163+
}
164+
165+
/**
166+
* Holds if broadcast doesn't specify receiving package name of the 3rd party app
167+
*/
168+
override predicate isSanitizer(DataFlow::Node node) {
169+
exists(DataFlow::Node intent | isCleanIntent(intent.asExpr()) |
170+
DataFlow::localFlow(intent, node)
171+
)
172+
}
173+
174+
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
175+
super.allowImplicitRead(node, c)
176+
or
177+
this.isSink(node)
178+
}
179+
}

java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.qhelp renamed to java/ql/src/Security/CWE/CWE-927/SensitiveCommunication.qhelp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,24 @@
22
<qhelp>
33

44
<overview>
5-
<p>Broadcast intents in an Android application are visible to all applications installed on the same mobile device, exposing all sensitive information they contain.</p>
6-
<p>Broadcasts are vulnerable to passive eavesdropping or active denial of service attacks when an intent is broadcast without specifying any receiver permission or receiver application.</p>
5+
<p>When an implicit Intent is used with a method such as <code>startActivity</code>, <code>startService</code>, or <code>sendBroadcast</code>, it may be read by other applications on the device.</p>
6+
<p>This means that sensitive data in these Intents may be leaked.</p>
77
</overview>
88

99
<recommendation>
10-
<p>
11-
Specify a receiver permission or application when broadcasting intents, or switch to
12-
<code>LocalBroadcastManager</code>
13-
or the latest
14-
<code>LiveData</code>
15-
library.
10+
<p>
11+
For <code>sendBroadcast</code> methods, a receiver permission may be specified so that only applications with a certain permission may receive the Intent;
12+
or a <code>LocalBroadcastManager</code> may be used.
13+
Otherwise, ensure that Intents containing sensitive data have an explicit receiver class set.
1614
</p>
1715
</recommendation>
1816

1917
<example>
20-
<p>The following example shows two ways of broadcasting intents. In the 'BAD' case, no "receiver permission" is specified. In the 'GOOD' case, "receiver permission" or "receiver application" is specified.</p>
21-
<sample src="SensitiveBroadcast.java" />
18+
<p>The following example shows two ways of broadcasting Intents. In the 'BAD' case, no "receiver permission" is specified. In the 'GOOD' case, "receiver permission" or "receiver application" is specified.</p>
19+
<sample src="SensitiveCommunication.java" />
2220
</example>
2321

2422
<references>
25-
<li>
26-
CWE:
27-
<a href="https://cwe.mitre.org/data/definitions/927.html">CWE-927: Use of Implicit Intent for Sensitive Communication</a>
28-
</li>
2923
<li>
3024
Android Developers:
3125
<a href="https://developer.android.com/guide/components/broadcasts">Security considerations and best practices for sending and receiving broadcasts</a>
@@ -46,5 +40,9 @@
4640
Android Developers:
4741
<a href="https://developer.android.com/topic/libraries/architecture/livedata">Android LiveData Overview</a>
4842
</li>
43+
<li>
44+
Oversecured:
45+
<a href="https://blog.oversecured.com/Interception-of-Android-implicit-intents/">Interception of Android implicit intents</a>
46+
</li>
4947
</references>
5048
</qhelp>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Leaking sensitive information through an implicit Intent
3+
* @description An Android application uses implicit Intents containing sensitive data
4+
* in a way that exposes it to arbitrary applications on the device.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 8.2
8+
* @precision medium
9+
* @id java/android/sensitive-communication
10+
* @tags security
11+
* external/cwe/cwe-927
12+
*/
13+
14+
import java
15+
import semmle.code.java.security.AndroidSensitiveCommunicationQuery
16+
import DataFlow::PathGraph
17+
18+
from SensitiveCommunicationConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
19+
where cfg.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink, "This call may leak sensitive information from $@.",
21+
source.getNode(), "here"

0 commit comments

Comments
 (0)