Skip to content

Commit 22b4df0

Browse files
aschackmullaschackmull
authored
Merge pull request #4512 from luchua-bc/sensitive-broadcast
Java: Sensitive broadcast
2 parents 9249444 + 2649522 commit 22b4df0

File tree

9 files changed

+474
-6
lines changed

9 files changed

+474
-6
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-532/SensitiveInfoLog.ql‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,21 @@
99

1010
import java
1111
import semmle.code.java.dataflow.TaintTracking
12+
import semmle.code.java.security.SensitiveActions
1213
import DataFlow
1314
import PathGraph
1415

1516
/**
16-
* Gets a regular expression for matching names of variables that indicate the value being held is a credential
17+
* Gets a regular expression for matching names of variables that indicate the value being held may contain sensitive information
1718
*/
18-
private string getACredentialRegex() {
19-
result = "(?i).*challenge|pass(wd|word|code|phrase)(?!.*question).*" or
20-
result = "(?i)(.*username|.*secret|url).*"
21-
}
19+
private string getACredentialRegex() { result = "(?i)(.*username|url).*" }
2220

2321
/** Variable keeps sensitive information judging by its name * */
2422
class CredentialExpr extends Expr {
2523
CredentialExpr() {
26-
exists(Variable v | this = v.getAnAccess() | v.getName().regexpMatch(getACredentialRegex()))
24+
exists(Variable v | this = v.getAnAccess() |
25+
v.getName().regexpMatch([getCommonSensitiveInfoRegex(), getACredentialRegex()])
26+
)
2727
}
2828
}
2929

‎java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
public void sendBroadcast1(Context context, String token, String refreshToken)
2+
{
3+
{
4+
// BAD: broadcast sensitive information to all listeners
5+
Intent intent = new Intent();
6+
intent.setAction("com.example.custom_action");
7+
intent.putExtra("token", token);
8+
intent.putExtra("refreshToken", refreshToken);
9+
context.sendBroadcast(intent);
10+
}
11+
12+
{
13+
// GOOD: broadcast sensitive information only to those with permission
14+
Intent intent = new Intent();
15+
intent.setAction("com.example.custom_action");
16+
intent.putExtra("token", token);
17+
intent.putExtra("refreshToken", refreshToken);
18+
context.sendBroadcast(intent, "com.example.user_permission");
19+
}
20+
21+
{
22+
// GOOD: broadcast sensitive information to a specific application
23+
Intent intent = new Intent();
24+
intent.setAction("com.example.custom_action");
25+
intent.setClassName("com.example2", "com.example2.UserInfoHandler");
26+
intent.putExtra("token", token);
27+
intent.putExtra("refreshToken", refreshToken);
28+
context.sendBroadcast(intent);
29+
}
30+
}

‎java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.qhelp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<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>
7+
</overview>
8+
9+
<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.
16+
</p>
17+
</recommendation>
18+
19+
<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" />
22+
</example>
23+
24+
<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>
29+
<li>
30+
Android Developers:
31+
<a href="https://developer.android.com/guide/components/broadcasts">Security considerations and best practices for sending and receiving broadcasts</a>
32+
</li>
33+
<li>
34+
SonarSource:
35+
<a href="https://rules.sonarsource.com/java/type/Security%20Hotspot/RSPEC-5320">Broadcasting intents is security-sensitive</a>
36+
</li>
37+
<li>
38+
Android Developer Fundamentals:
39+
<a href="https://google-developer-training.github.io/android-developer-fundamentals-course-concepts-v2/unit-3-working-in-the-background/lesson-7-background-tasks/7-3-c-broadcasts/7-3-c-broadcasts.html">Restricting broadcasts</a>
40+
</li>
41+
<li>
42+
Carnegie Mellon University:
43+
<a href="https://wiki.sei.cmu.edu/confluence/display/android/DRD03-J.+Do+not+broadcast+sensitive+information+using+an+implicit+intent">DRD03-J. Do not broadcast sensitive information using an implicit intent</a>
44+
</li>
45+
<li>
46+
Android Developers:
47+
<a href="https://developer.android.com/topic/libraries/architecture/livedata">Android LiveData Overview</a>
48+
</li>
49+
</references>
50+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/**
2+
* @name Broadcasting sensitive data to all Android applications
3+
* @id java/sensitive-broadcast
4+
* @description An Android application uses implicit intents to broadcast sensitive data to all applications without specifying any receiver permission.
5+
* @kind path-problem
6+
* @tags security
7+
* external/cwe-927
8+
*/
9+
10+
import java
11+
import semmle.code.java.dataflow.DataFlow3
12+
import semmle.code.java.dataflow.TaintTracking
13+
import semmle.code.java.frameworks.android.Intent
14+
import semmle.code.java.security.SensitiveActions
15+
import DataFlow::PathGraph
16+
17+
/**
18+
* Gets regular expression for matching names of Android variables that indicate the value being held contains sensitive information.
19+
*/
20+
private string getAndroidSensitiveInfoRegex() { result = "(?i).*(email|phone|ticket).*" }
21+
22+
/**
23+
* Method call to pass information to the `Intent` object.
24+
*/
25+
class PutIntentExtraMethodAccess extends MethodAccess {
26+
PutIntentExtraMethodAccess() {
27+
(
28+
getMethod().getName().matches("put%Extra") or
29+
getMethod().hasName("putExtras")
30+
) and
31+
getMethod().getDeclaringType() instanceof TypeIntent
32+
}
33+
}
34+
35+
/**
36+
* Method call to pass information to the intent extra bundle object.
37+
*/
38+
class PutBundleExtraMethodAccess extends MethodAccess {
39+
PutBundleExtraMethodAccess() {
40+
getMethod().getName().regexpMatch("put\\w+") and
41+
getMethod().getDeclaringType().getASupertype*().hasQualifiedName("android.os", "BaseBundle")
42+
}
43+
}
44+
45+
/** Finds variables that hold sensitive information judging by their names. */
46+
class SensitiveInfoExpr extends Expr {
47+
SensitiveInfoExpr() {
48+
exists(Variable v | this = v.getAnAccess() |
49+
v.getName().regexpMatch([getCommonSensitiveInfoRegex(), getAndroidSensitiveInfoRegex()])
50+
)
51+
}
52+
}
53+
54+
/**
55+
* A method access of the `Context.sendBroadcast` family.
56+
*/
57+
class SendBroadcastMethodAccess extends MethodAccess {
58+
SendBroadcastMethodAccess() {
59+
this.getMethod().getDeclaringType() instanceof TypeContext and
60+
this.getMethod().getName().matches("send%Broadcast%")
61+
}
62+
}
63+
64+
private class NullArgFlowConfig extends DataFlow2::Configuration {
65+
NullArgFlowConfig() { this = "Flow configuration with a null argument" }
66+
67+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof NullLiteral }
68+
69+
override predicate isSink(DataFlow::Node sink) {
70+
exists(SendBroadcastMethodAccess ma | sink.asExpr() = ma.getAnArgument())
71+
}
72+
}
73+
74+
private class EmptyArrayArgFlowConfig extends DataFlow3::Configuration {
75+
EmptyArrayArgFlowConfig() { this = "Flow configuration with an empty array argument" }
76+
77+
override predicate isSource(DataFlow::Node src) {
78+
src.asExpr().(ArrayCreationExpr).getFirstDimensionSize() = 0
79+
}
80+
81+
override predicate isSink(DataFlow::Node sink) {
82+
exists(SendBroadcastMethodAccess ma | sink.asExpr() = ma.getAnArgument())
83+
}
84+
}
85+
86+
/**
87+
* Holds if a `sendBroadcast` call doesn't specify receiver permission.
88+
*/
89+
predicate isSensitiveBroadcastSink(DataFlow::Node sink) {
90+
exists(SendBroadcastMethodAccess ma |
91+
sink.asExpr() = ma.getAnArgument() and
92+
(
93+
ma.getMethod().hasName("sendBroadcast") and
94+
(
95+
ma.getNumArgument() = 1 // sendBroadcast(Intent intent)
96+
or
97+
// sendBroadcast(Intent intent, String receiverPermission)
98+
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(1))))
99+
)
100+
or
101+
ma.getMethod().hasName("sendBroadcastAsUser") and
102+
(
103+
ma.getNumArgument() = 2 or // sendBroadcastAsUser(Intent intent, UserHandle user)
104+
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2)))) // sendBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission)
105+
)
106+
or
107+
ma.getMethod().hasName("sendBroadcastWithMultiplePermissions") and
108+
exists(EmptyArrayArgFlowConfig config |
109+
config.hasFlow(_, DataFlow::exprNode(ma.getArgument(1))) // sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions)
110+
)
111+
or
112+
// Method calls of `sendOrderedBroadcast` whose second argument is always `receiverPermission`
113+
ma.getMethod().hasName("sendOrderedBroadcast") and
114+
(
115+
// sendOrderedBroadcast(Intent intent, String receiverPermission) or sendOrderedBroadcast(Intent intent, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
116+
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(1)))) and
117+
ma.getNumArgument() <= 7
118+
or
119+
// sendOrderedBroadcast(Intent intent, String receiverPermission, String receiverAppOp, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
120+
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(1)))) and
121+
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2)))) and
122+
ma.getNumArgument() = 8
123+
)
124+
or
125+
// Method call of `sendOrderedBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)`
126+
ma.getMethod().hasName("sendOrderedBroadcastAsUser") and
127+
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2))))
128+
)
129+
)
130+
}
131+
132+
/**
133+
* Taint configuration tracking flow from variables containing sensitive information to broadcast intents.
134+
*/
135+
class SensitiveBroadcastConfig extends TaintTracking::Configuration {
136+
SensitiveBroadcastConfig() { this = "Sensitive Broadcast Configuration" }
137+
138+
override predicate isSource(DataFlow::Node source) {
139+
source.asExpr() instanceof SensitiveInfoExpr
140+
}
141+
142+
override predicate isSink(DataFlow::Node sink) { isSensitiveBroadcastSink(sink) }
143+
144+
/**
145+
* Holds if there is an additional flow step from `PutIntentExtraMethodAccess` or `PutBundleExtraMethodAccess` that taints the `Intent` or its extras `Bundle`.
146+
*/
147+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
148+
exists(PutIntentExtraMethodAccess pia |
149+
node1.asExpr() = pia.getAnArgument() and node2.asExpr() = pia.getQualifier()
150+
)
151+
or
152+
exists(PutBundleExtraMethodAccess pba |
153+
node1.asExpr() = pba.getAnArgument() and node2.asExpr() = pba.getQualifier()
154+
)
155+
}
156+
157+
/**
158+
* Holds if broadcast doesn't specify receiving package name of the 3rd party app
159+
*/
160+
override predicate isSanitizer(DataFlow::Node node) {
161+
exists(MethodAccess setReceiverMa |
162+
setReceiverMa.getMethod().hasName(["setPackage", "setClass", "setClassName", "setComponent"]) and
163+
setReceiverMa.getQualifier().(VarAccess).getVariable().getAnAccess() = node.asExpr()
164+
)
165+
}
166+
}
167+
168+
from SensitiveBroadcastConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
169+
where cfg.hasFlowPath(source, sink)
170+
select sink.getNode(), source, sink, "Sending $@ to broadcast.", source.getNode(),
171+
"sensitive information"

‎java/ql/src/semmle/code/java/security/SensitiveActions.qll‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ private string nonSuspicious() {
2727
result = "%crypt%"
2828
}
2929

30+
/**
31+
* Gets a regular expression for matching common names of variables that indicate the value being held contains sensitive information.
32+
*/
33+
string getCommonSensitiveInfoRegex() {
34+
result = "(?i).*challenge|pass(wd|word|code|phrase)(?!.*question).*" or
35+
result = "(?i).*(token|secret).*"
36+
}
37+
3038
/** An expression that might contain sensitive data. */
3139
abstract class SensitiveExpr extends Expr { }
3240

‎java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
edges
2+
| SensitiveBroadcast.java:12:34:12:38 | token : String | SensitiveBroadcast.java:14:31:14:36 | intent |
3+
| SensitiveBroadcast.java:13:41:13:52 | refreshToken : String | SensitiveBroadcast.java:14:31:14:36 | intent |
4+
| SensitiveBroadcast.java:25:32:25:39 | password : String | SensitiveBroadcast.java:26:31:26:36 | intent |
5+
| SensitiveBroadcast.java:36:35:36:39 | email : String | SensitiveBroadcast.java:38:31:38:36 | intent |
6+
| SensitiveBroadcast.java:50:22:50:29 | password : String | SensitiveBroadcast.java:52:31:52:36 | intent |
7+
| SensitiveBroadcast.java:97:35:97:40 | ticket : String | SensitiveBroadcast.java:98:54:98:59 | intent |
8+
| SensitiveBroadcast.java:109:32:109:39 | passcode : String | SensitiveBroadcast.java:111:54:111:59 | intent |
9+
| SensitiveBroadcast.java:136:33:136:38 | passwd : String | SensitiveBroadcast.java:140:54:140:59 | intent |
10+
nodes
11+
| SensitiveBroadcast.java:12:34:12:38 | token : String | semmle.label | token : String |
12+
| SensitiveBroadcast.java:13:41:13:52 | refreshToken : String | semmle.label | refreshToken : String |
13+
| SensitiveBroadcast.java:14:31:14:36 | intent | semmle.label | intent |
14+
| SensitiveBroadcast.java:25:32:25:39 | password : String | semmle.label | password : String |
15+
| SensitiveBroadcast.java:26:31:26:36 | intent | semmle.label | intent |
16+
| SensitiveBroadcast.java:36:35:36:39 | email : String | semmle.label | email : String |
17+
| SensitiveBroadcast.java:38:31:38:36 | intent | semmle.label | intent |
18+
| SensitiveBroadcast.java:50:22:50:29 | password : String | semmle.label | password : String |
19+
| SensitiveBroadcast.java:52:31:52:36 | intent | semmle.label | intent |
20+
| SensitiveBroadcast.java:97:35:97:40 | ticket : String | semmle.label | ticket : String |
21+
| SensitiveBroadcast.java:98:54:98:59 | intent | semmle.label | intent |
22+
| SensitiveBroadcast.java:109:32:109:39 | passcode : String | semmle.label | passcode : String |
23+
| SensitiveBroadcast.java:111:54:111:59 | intent | semmle.label | intent |
24+
| SensitiveBroadcast.java:136:33:136:38 | passwd : String | semmle.label | passwd : String |
25+
| SensitiveBroadcast.java:140:54:140:59 | intent | semmle.label | intent |
26+
#select
27+
| SensitiveBroadcast.java:14:31:14:36 | intent | SensitiveBroadcast.java:12:34:12:38 | token : String | SensitiveBroadcast.java:14:31:14:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:12:34:12:38 | token | sensitive information |
28+
| SensitiveBroadcast.java:14:31:14:36 | intent | SensitiveBroadcast.java:13:41:13:52 | refreshToken : String | SensitiveBroadcast.java:14:31:14:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:13:41:13:52 | refreshToken | sensitive information |
29+
| SensitiveBroadcast.java:26:31:26:36 | intent | SensitiveBroadcast.java:25:32:25:39 | password : String | SensitiveBroadcast.java:26:31:26:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:25:32:25:39 | password | sensitive information |
30+
| SensitiveBroadcast.java:38:31:38:36 | intent | SensitiveBroadcast.java:36:35:36:39 | email : String | SensitiveBroadcast.java:38:31:38:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:36:35:36:39 | email | sensitive information |
31+
| SensitiveBroadcast.java:52:31:52:36 | intent | SensitiveBroadcast.java:50:22:50:29 | password : String | SensitiveBroadcast.java:52:31:52:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:50:22:50:29 | password | sensitive information |
32+
| SensitiveBroadcast.java:98:54:98:59 | intent | SensitiveBroadcast.java:97:35:97:40 | ticket : String | SensitiveBroadcast.java:98:54:98:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:97:35:97:40 | ticket | sensitive information |
33+
| SensitiveBroadcast.java:111:54:111:59 | intent | SensitiveBroadcast.java:109:32:109:39 | passcode : String | SensitiveBroadcast.java:111:54:111:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:109:32:109:39 | passcode | sensitive information |
34+
| SensitiveBroadcast.java:140:54:140:59 | intent | SensitiveBroadcast.java:136:33:136:38 | passwd : String | SensitiveBroadcast.java:140:54:140:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:136:33:136:38 | passwd | sensitive information |

0 commit comments

Comments
 (0)