diff --git a/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.java b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.java new file mode 100644 index 00000000000..e9fd7b15dc1 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.java @@ -0,0 +1,20 @@ +public void sendBroadcast1(Context context, String token, String refreshToken) +{ + { + // BAD: broadcast sensitive information without permission + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + intent.putExtra("token", token); + intent.putExtra("refreshToken", refreshToken); + context.sendBroadcast(intent); + } + + { + // GOOD: broadcast sensitive information with permission + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + intent.putExtra("token", token); + intent.putExtra("refreshToken", refreshToken); + context.sendBroadcast(intent, "com.example.user_permission"); + } +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.qhelp b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.qhelp new file mode 100644 index 00000000000..b9acfd7b7c9 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.qhelp @@ -0,0 +1,25 @@ + + + + +

Broadcasted intents in an Android application are visible to all applications installed on the same mobile device, exposing all sensitive information they contain.

+

Broadcasts are vulnerable to passive eavesdropping or active denial of service attacks when an intent is broadcasted without specifying any receiver permission or receiver application.

+
+ + +

Specify receiver permission or specify receiver application in broadcasted intents, or switch to LocalBroadcastManager or the latest LiveData library.

+
+ + +

The following example shows two ways of broadcasting intents. In the 'BAD' case, no "receiver permission" is specified. In the 'GOOD' case, "receiver permission" is specified.

+ +
+ + +
  • +CWE-927: Use of Implicit Intent for Sensitive Communication +
  • +
    +
    \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql new file mode 100644 index 00000000000..9b024a9866e --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql @@ -0,0 +1,120 @@ +/** + * @name Use of Implicit Intent for Sensitive Communication + * @id java/sensitive-broadcast + * @description An Android application uses implicit intents to broadcast sensitive data to all applications without specifying any receiver permission. + * @kind path-problem + * @tags security + * external/cwe-927 + */ + +import java +import semmle.code.java.frameworks.android.Intent +import semmle.code.java.dataflow.TaintTracking +import DataFlow +import PathGraph + +/** + * Gets a regular expression for matching names of variables that indicate the value being held contains sensitive information. + */ +private string getSensitiveInfoRegex() { + result = "(?i).*challenge|pass(wd|word|code|phrase)(?!.*question).*" or + result = "(?i).*(token|email|phone|username|userid|ticket).*" +} + +/** + * Method call to pass information to the `Intent` object either directly through intent extra or indirectly through intent extra bundle. + */ +class PutExtraMethodAccess extends MethodAccess { + PutExtraMethodAccess() { + getMethod().getName().regexpMatch("put\\w*Extra(s*)") and + getMethod().getDeclaringType() instanceof TypeIntent and + not exists( + MethodAccess setPackageVa // Intent without specifying receiving package name of the 3rd party app + | + setPackageVa.getMethod().hasName(["setPackage", "setClass", "setClassName", "setComponent"]) and + setPackageVa.getQualifier().(VarAccess).getVariable().getAnAccess() = getQualifier() + ) + or + getMethod().getName().regexpMatch("put\\w+") and + getMethod().getDeclaringType().hasQualifiedName("android.os", "Bundle") + } +} + +/** Finds variables that hold sensitive information judging by their names. */ +class SensitiveInfoExpr extends Expr { + SensitiveInfoExpr() { + exists(Variable v | this = v.getAnAccess() | + v.getName().toLowerCase().regexpMatch(getSensitiveInfoRegex()) + ) + } +} + +/** + * The method access of `context.sendBroadcast`. + */ +class SendBroadcastMethodAccess extends MethodAccess { + SendBroadcastMethodAccess() { + this.getMethod().getDeclaringType() instanceof TypeContext and + this.getMethod().getName().matches("send%Broadcast%") + } +} + +/** + * 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 or // sendBroadcast(Intent intent) + ma.getArgument(1) instanceof NullLiteral // sendBroadcast(Intent intent, String receiverPermission) + ) + or + ma.getMethod().hasName("sendBroadcastAsUser") and + ( + ma.getNumArgument() = 2 or // sendBroadcastAsUser(Intent intent, UserHandle user) + ma.getArgument(2) instanceof NullLiteral // sendBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission) + ) + or + ma.getMethod().hasName("sendBroadcastWithMultiplePermissions") and + ma.getArgument(1) instanceof NullLiteral // sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions) + or + //Method calls of `sendOrderedBroadcast` whose second argument is always `receiverPermission` + ma.getMethod().hasName("sendOrderedBroadcast") and + ma.getArgument(1) instanceof NullLiteral + 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 + ma.getArgument(2) instanceof NullLiteral + ) + ) +} + +/** + * Taint configuration tracking flow from variables containing sensitive information to broadcasted intents. + */ +class SensitiveBroadcastConfig extends DataFlow::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 `PutExtraMethodAccess` to a broadcasted intent. + */ + override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(PutExtraMethodAccess pa | + node1.asExpr() = pa.getAnArgument() and node2.asExpr() = pa.getQualifier() + ) + } +} + +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" diff --git a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected new file mode 100644 index 00000000000..163a34e6a49 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected @@ -0,0 +1,24 @@ +edges +| SensitiveBroadcast.java:11:34:11:38 | token : String | SensitiveBroadcast.java:13:31:13:36 | intent | +| SensitiveBroadcast.java:12:41:12:52 | refreshToken : String | SensitiveBroadcast.java:13:31:13:36 | intent | +| SensitiveBroadcast.java:23:33:23:40 | username : String | SensitiveBroadcast.java:25:31:25:36 | intent | +| SensitiveBroadcast.java:24:32:24:39 | password : String | SensitiveBroadcast.java:25:31:25:36 | intent | +| SensitiveBroadcast.java:36:40:36:47 | username : String | SensitiveBroadcast.java:39:31:39:36 | intent | +| SensitiveBroadcast.java:37:39:37:46 | password : String | SensitiveBroadcast.java:39:31:39:36 | intent | +nodes +| SensitiveBroadcast.java:11:34:11:38 | token : String | semmle.label | token : String | +| SensitiveBroadcast.java:12:41:12:52 | refreshToken : String | semmle.label | refreshToken : String | +| SensitiveBroadcast.java:13:31:13:36 | intent | semmle.label | intent | +| SensitiveBroadcast.java:23:33:23:40 | username : String | semmle.label | username : String | +| SensitiveBroadcast.java:24:32:24:39 | password : String | semmle.label | password : String | +| SensitiveBroadcast.java:25:31:25:36 | intent | semmle.label | intent | +| SensitiveBroadcast.java:36:40:36:47 | username : String | semmle.label | username : String | +| SensitiveBroadcast.java:37:39:37:46 | password : String | semmle.label | password : String | +| SensitiveBroadcast.java:39:31:39:36 | intent | semmle.label | intent | +#select +| SensitiveBroadcast.java:13:31:13:36 | intent | SensitiveBroadcast.java:11:34:11:38 | token : String | SensitiveBroadcast.java:13:31:13:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:11:34:11:38 | token | sensitive information | +| SensitiveBroadcast.java:13:31:13:36 | intent | SensitiveBroadcast.java:12:41:12:52 | refreshToken : String | SensitiveBroadcast.java:13:31:13:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:12:41:12:52 | refreshToken | sensitive information | +| SensitiveBroadcast.java:25:31:25:36 | intent | SensitiveBroadcast.java:23:33:23:40 | username : String | SensitiveBroadcast.java:25:31:25:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:23:33:23:40 | username | sensitive information | +| SensitiveBroadcast.java:25:31:25:36 | intent | SensitiveBroadcast.java:24:32:24:39 | password : String | SensitiveBroadcast.java:25:31:25:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:24:32:24:39 | password | sensitive information | +| SensitiveBroadcast.java:39:31:39:36 | intent | SensitiveBroadcast.java:36:40:36:47 | username : String | SensitiveBroadcast.java:39:31:39:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:36:40:36:47 | username | sensitive information | +| SensitiveBroadcast.java:39:31:39:36 | intent | SensitiveBroadcast.java:37:39:37:46 | password : String | SensitiveBroadcast.java:39:31:39:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:37:39:37:46 | password | sensitive information | diff --git a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java new file mode 100644 index 00000000000..c84e709a8d6 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java @@ -0,0 +1,66 @@ +import android.content.Context; +import android.content.Intent; +import android.os.Bundle; + +class SensitiveBroadcast { + + //Tests broadcast of access token with intent extra. + public void sendBroadcast1(Context context, String token, String refreshToken) { + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + intent.putExtra("token", token); + intent.putExtra("refreshToken", refreshToken); + context.sendBroadcast(intent); + } + + //Tests broadcast of sensitive user information with intent extra. + public void sendBroadcast2(Context context) { + String username = "test123"; + String password = "abc12345"; + + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + intent.putExtra("name", username); + intent.putExtra("pwd", password); + context.sendBroadcast(intent); + } + + //Tests broadcast of sensitive user information with extra bundle. + public void sendBroadcast3(Context context) { + String username = "test123"; + String password = "abc12345"; + + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + Bundle bundle = new Bundle(); + bundle.putCharSequence("name", username); + bundle.putCharSequence("pwd", password); + intent.putExtras(bundle); + context.sendBroadcast(intent); + } + + //Tests broadcast of sensitive user information with permission. + public void sendBroadcast4(Context context) { + String username = "test123"; + String password = "abc12345"; + + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + intent.putExtra("name", username); + intent.putExtra("pwd", password); + context.sendBroadcast(intent, "com.example.user_permission"); + } + + //Tests broadcast of sensitive user information to a specific application. + public void sendBroadcast5(Context context) { + String username = "test123"; + String password = "abc12345"; + + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + intent.setClassName("com.example2", "com.example2.UserInfoHandler"); + intent.putExtra("name", username); + intent.putExtra("pwd", password); + context.sendBroadcast(intent); + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.qlref b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.qlref new file mode 100644 index 00000000000..20c5250d80d --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-927/options b/java/ql/test/experimental/query-tests/security/CWE-927/options new file mode 100644 index 00000000000..43e25f608b6 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-927/options @@ -0,0 +1 @@ +// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0 diff --git a/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java b/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java index e3ef6277dba..292069c0f71 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java @@ -1996,4 +1996,77 @@ public class Intent implements Parcelable, Cloneable { public void readFromParcel(Parcel in) { } + + /** + * Retrieve the application package name this Intent is limited to. When + * resolving an Intent, if non-null this limits the resolution to only + * components in the given application package. + * + * @return The name of the application package for the Intent. + * + * @see #resolveActivity + * @see #setPackage + */ + public String getPackage() { + return null; + } + + /** + * (Usually optional) Set an explicit application package name that limits + * the components this Intent will resolve to. If left to the default + * value of null, all components in all applications will considered. + * If non-null, the Intent can only match the components in the given + * application package. + * + * @param packageName The name of the application package to handle the + * intent, or null to allow any application package. + * + * @return Returns the same Intent object, for chaining multiple calls + * into a single statement. + * + * @see #getPackage + * @see #resolveActivity + */ + public Intent setPackage(String packageName) { + return null; + } + + /** + * Convenience for calling {@link #setComponent} with an + * explicit class name. + * + * @param packageContext A Context of the application package implementing + * this class. + * @param className The name of a class inside of the application package + * that will be used as the component for this Intent. + * + * @return Returns the same Intent object, for chaining multiple calls + * into a single statement. + * + * @see #setComponent + * @see #setClass + */ + public Intent setClassName(Context packageContext, String className) { + return null; + } + + /** + * Convenience for calling {@link #setComponent} with an + * explicit application package name and class name. + * + * @param packageName The name of the package implementing the desired + * component. + * @param className The name of a class inside of the application package + * that will be used as the component for this Intent. + * + * @return Returns the same Intent object, for chaining multiple calls + * into a single statement. + * + * @see #setComponent + * @see #setClass + */ + public Intent setClassName(String packageName, String className) { + return null; + } + } \ No newline at end of file