mirror of
https://github.com/github/codeql.git
synced 2025-12-21 11:16:30 +01:00
Merge pull request #6599 from joefarebrother/android-sensitive-communication
Java: Promote android sensitive broadcast query
This commit is contained in:
Joe Farebrother
@@ -2,30 +2,24 @@
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<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>
|
||||
<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>
|
||||
<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>
|
||||
<p>This means that sensitive data in these Intents may be leaked.</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Specify a receiver permission or application when broadcasting intents, or switch to
|
||||
<code>LocalBroadcastManager</code>
|
||||
or the latest
|
||||
<code>LiveData</code>
|
||||
library.
|
||||
<p>
|
||||
For <code>sendBroadcast</code> methods, a receiver permission may be specified so that only applications with a certain permission may receive the Intent;
|
||||
or a <code>LocalBroadcastManager</code> may be used.
|
||||
Otherwise, ensure that Intents containing sensitive data have an explicit receiver class set.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<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>
|
||||
<sample src="SensitiveBroadcast.java" />
|
||||
<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>
|
||||
<sample src="SensitiveCommunication.java" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>
|
||||
CWE:
|
||||
<a href="https://cwe.mitre.org/data/definitions/927.html">CWE-927: Use of Implicit Intent for Sensitive Communication</a>
|
||||
</li>
|
||||
<li>
|
||||
Android Developers:
|
||||
<a href="https://developer.android.com/guide/components/broadcasts">Security considerations and best practices for sending and receiving broadcasts</a>
|
||||
@@ -46,5 +40,9 @@
|
||||
Android Developers:
|
||||
<a href="https://developer.android.com/topic/libraries/architecture/livedata">Android LiveData Overview</a>
|
||||
</li>
|
||||
<li>
|
||||
Oversecured:
|
||||
<a href="https://blog.oversecured.com/Interception-of-Android-implicit-intents/">Interception of Android implicit intents</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
21
java/ql/src/Security/CWE/CWE-927/SensitiveCommunication.ql
Normal file
21
java/ql/src/Security/CWE/CWE-927/SensitiveCommunication.ql
Normal file
@@ -0,0 +1,21 @@
|
||||
/**
|
||||
* @name Leaking sensitive information through an implicit Intent
|
||||
* @description An Android application uses implicit Intents containing sensitive data
|
||||
* in a way that exposes it to arbitrary applications on the device.
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 8.2
|
||||
* @precision medium
|
||||
* @id java/android/sensitive-communication
|
||||
* @tags security
|
||||
* external/cwe/cwe-927
|
||||
*/
|
||||
|
||||
import java
|
||||
import semmle.code.java.security.AndroidSensitiveCommunicationQuery
|
||||
import DataFlow::PathGraph
|
||||
|
||||
from SensitiveCommunicationConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where cfg.hasFlowPath(source, sink)
|
||||
select sink.getNode(), source, sink, "This call may leak sensitive information from $@.",
|
||||
source.getNode(), "here"
|
||||
@@ -1,175 +0,0 @@
|
||||
/**
|
||||
* @name Broadcasting sensitive data to all Android applications
|
||||
* @description An Android application uses implicit intents to broadcast
|
||||
* sensitive data to all applications without specifying any
|
||||
* receiver permission.
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @precision medium
|
||||
* @id java/sensitive-broadcast
|
||||
* @tags security
|
||||
* external/cwe/cwe-927
|
||||
*/
|
||||
|
||||
import java
|
||||
import semmle.code.java.dataflow.DataFlow3
|
||||
import semmle.code.java.dataflow.TaintTracking
|
||||
import semmle.code.java.frameworks.android.Intent
|
||||
import semmle.code.java.security.SensitiveActions
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* Gets regular expression for matching names of Android variables that indicate the value being held contains sensitive information.
|
||||
*/
|
||||
private string getAndroidSensitiveInfoRegex() { result = "(?i).*(email|phone|ticket).*" }
|
||||
|
||||
/**
|
||||
* Method call to pass information to the `Intent` object.
|
||||
*/
|
||||
class PutIntentExtraMethodAccess extends MethodAccess {
|
||||
PutIntentExtraMethodAccess() {
|
||||
(
|
||||
getMethod().getName().matches("put%Extra") or
|
||||
getMethod().hasName("putExtras")
|
||||
) and
|
||||
getMethod().getDeclaringType() instanceof TypeIntent
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Method call to pass information to the intent extra bundle object.
|
||||
*/
|
||||
class PutBundleExtraMethodAccess extends MethodAccess {
|
||||
PutBundleExtraMethodAccess() {
|
||||
getMethod().getName().regexpMatch("put\\w+") and
|
||||
getMethod().getDeclaringType().getASupertype*().hasQualifiedName("android.os", "BaseBundle")
|
||||
}
|
||||
}
|
||||
|
||||
/** Finds variables that hold sensitive information judging by their names. */
|
||||
class SensitiveInfoExpr extends Expr {
|
||||
SensitiveInfoExpr() {
|
||||
exists(Variable v | this = v.getAnAccess() |
|
||||
v.getName().regexpMatch([getCommonSensitiveInfoRegex(), getAndroidSensitiveInfoRegex()])
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A method access of the `Context.sendBroadcast` family.
|
||||
*/
|
||||
class SendBroadcastMethodAccess extends MethodAccess {
|
||||
SendBroadcastMethodAccess() {
|
||||
this.getMethod().getDeclaringType() instanceof TypeContext and
|
||||
this.getMethod().getName().matches("send%Broadcast%")
|
||||
}
|
||||
}
|
||||
|
||||
private class NullArgFlowConfig extends DataFlow2::Configuration {
|
||||
NullArgFlowConfig() { this = "Flow configuration with a null argument" }
|
||||
|
||||
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof NullLiteral }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(SendBroadcastMethodAccess ma | sink.asExpr() = ma.getAnArgument())
|
||||
}
|
||||
}
|
||||
|
||||
private class EmptyArrayArgFlowConfig extends DataFlow3::Configuration {
|
||||
EmptyArrayArgFlowConfig() { this = "Flow configuration with an empty array argument" }
|
||||
|
||||
override predicate isSource(DataFlow::Node src) {
|
||||
src.asExpr().(ArrayCreationExpr).getFirstDimensionSize() = 0
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(SendBroadcastMethodAccess ma | sink.asExpr() = ma.getAnArgument())
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if a `sendBroadcast` call doesn't specify receiver permission.
|
||||
*/
|
||||
predicate isSensitiveBroadcastSink(DataFlow::Node sink) {
|
||||
exists(SendBroadcastMethodAccess ma |
|
||||
sink.asExpr() = ma.getAnArgument() and
|
||||
(
|
||||
ma.getMethod().hasName("sendBroadcast") and
|
||||
(
|
||||
ma.getNumArgument() = 1 // sendBroadcast(Intent intent)
|
||||
or
|
||||
// sendBroadcast(Intent intent, String receiverPermission)
|
||||
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(1))))
|
||||
)
|
||||
or
|
||||
ma.getMethod().hasName("sendBroadcastAsUser") and
|
||||
(
|
||||
ma.getNumArgument() = 2 or // sendBroadcastAsUser(Intent intent, UserHandle user)
|
||||
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2)))) // sendBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission)
|
||||
)
|
||||
or
|
||||
ma.getMethod().hasName("sendBroadcastWithMultiplePermissions") and
|
||||
exists(EmptyArrayArgFlowConfig config |
|
||||
config.hasFlow(_, DataFlow::exprNode(ma.getArgument(1))) // sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions)
|
||||
)
|
||||
or
|
||||
// Method calls of `sendOrderedBroadcast` whose second argument is always `receiverPermission`
|
||||
ma.getMethod().hasName("sendOrderedBroadcast") and
|
||||
(
|
||||
// sendOrderedBroadcast(Intent intent, String receiverPermission) or sendOrderedBroadcast(Intent intent, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
|
||||
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(1)))) and
|
||||
ma.getNumArgument() <= 7
|
||||
or
|
||||
// sendOrderedBroadcast(Intent intent, String receiverPermission, String receiverAppOp, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)
|
||||
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(1)))) and
|
||||
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2)))) and
|
||||
ma.getNumArgument() = 8
|
||||
)
|
||||
or
|
||||
// Method call of `sendOrderedBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)`
|
||||
ma.getMethod().hasName("sendOrderedBroadcastAsUser") and
|
||||
exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2))))
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Taint configuration tracking flow from variables containing sensitive information to broadcast intents.
|
||||
*/
|
||||
class SensitiveBroadcastConfig extends TaintTracking::Configuration {
|
||||
SensitiveBroadcastConfig() { this = "Sensitive Broadcast Configuration" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
source.asExpr() instanceof SensitiveInfoExpr
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { isSensitiveBroadcastSink(sink) }
|
||||
|
||||
/**
|
||||
* Holds if there is an additional flow step from `PutIntentExtraMethodAccess` or `PutBundleExtraMethodAccess` that taints the `Intent` or its extras `Bundle`.
|
||||
*/
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
exists(PutIntentExtraMethodAccess pia |
|
||||
node1.asExpr() = pia.getAnArgument() and node2.asExpr() = pia.getQualifier()
|
||||
)
|
||||
or
|
||||
exists(PutBundleExtraMethodAccess pba |
|
||||
node1.asExpr() = pba.getAnArgument() and node2.asExpr() = pba.getQualifier()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if broadcast doesn't specify receiving package name of the 3rd party app
|
||||
*/
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
exists(MethodAccess setReceiverMa |
|
||||
setReceiverMa.getMethod().hasName(["setPackage", "setClass", "setClassName", "setComponent"]) and
|
||||
setReceiverMa.getQualifier().(VarAccess).getVariable().getAnAccess() = node.asExpr()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
from SensitiveBroadcastConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where cfg.hasFlowPath(source, sink)
|
||||
select sink.getNode(), source, sink, "Sending $@ to broadcast.", source.getNode(),
|
||||
"sensitive information"
|
||||
Reference in New Issue
Block a user