Merge pull request #6966 from atorralba/atorralba/android-explicit-intent-sanitizer

Android: Add ExplicitIntentSanitizer and allowIntentExtrasImplicitRead

java/ql/lib/semmle/code/java/frameworks/android/Intent.qll

@@ -77,6 +77,61 @@ class IntentGetParcelableExtraMethod extends Method {
   }
 }
 
+/** The class `android.os.BaseBundle`, or a class that extends it. */
+class AndroidBundle extends Class {
+  AndroidBundle() { this.getASupertype*().hasQualifiedName("android.os", "BaseBundle") }
+}
+
+/** An `Intent` that explicitly sets a destination component. */
+class ExplicitIntent extends Expr {
+  ExplicitIntent() {
+    exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      m.getDeclaringType() instanceof TypeIntent and
+      m.hasName(["setPackage", "setClass", "setClassName", "setComponent"]) and
+      ma.getQualifier() = this
+    )
+    or
+    exists(ConstructorCall cc, Argument classArg |
+      cc.getConstructedType() instanceof TypeIntent and
+      cc.getAnArgument() = classArg and
+      classArg.getType() instanceof TypeClass and
+      not exists(NullLiteral nullLiteral | DataFlow::localExprFlow(nullLiteral, classArg)) and
+      cc = this
+    )
+  }
+}
+
+/**
+ * A sanitizer for explicit intents.
+ *
+ * Use this when you want to work only with implicit intents
+ * in a `DataFlow` or `TaintTracking` configuration.
+ */
+class ExplicitIntentSanitizer extends DataFlow::Node {
+  ExplicitIntentSanitizer() {
+    exists(ExplicitIntent explIntent | DataFlow::localExprFlow(explIntent, this.asExpr()))
+  }
+}
+
+private class BundleExtrasSyntheticField extends SyntheticField {
+  BundleExtrasSyntheticField() { this = "android.content.Intent.extras" }
+
+  override RefType getType() { result instanceof AndroidBundle }
+}
+
+/**
+ * Holds if extras may be implicitly read from the Intent `node`.
+ */
+predicate allowIntentExtrasImplicitRead(DataFlow::Node node, DataFlow::Content c) {
+  node.getType() instanceof TypeIntent and
+  (
+    c instanceof DataFlow::MapValueContent
+    or
+    c.(DataFlow::SyntheticFieldContent).getType() instanceof AndroidBundle
+  )
+}
+
 private class IntentBundleFlowSteps extends SummaryModelCsv {
   override predicate row(string row) {
     row =

java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll

@@ -121,31 +121,6 @@ private predicate isStartActivityOrServiceSink(DataFlow::Node arg) {
   )
 }
 
-private predicate isCleanIntent(Expr intent) {
-  intent.getType() instanceof TypeIntent and
-  (
-    exists(MethodAccess setRecieverMa |
-      setRecieverMa.getQualifier() = intent and
-      setRecieverMa.getMethod().hasName(["setPackage", "setClass", "setClassName", "setComponent"])
-    )
-    or
-    // Handle the cases where the PackageContext and Class are set at construction time
-    //    Intent(Context packageContext, Class<?> cls)
-    //    Intent(String action, Uri uri, Context packageContext, Class<?> cls)
-    exists(ConstructorCall cc | cc = intent |
-      cc.getConstructedType() instanceof TypeIntent and
-      cc.getNumArgument() > 1 and
-      (
-        cc.getArgument(0).getType() instanceof TypeContext and
-        not maybeNullArg(cc.getArgument(1))
-        or
-        cc.getArgument(2).getType() instanceof TypeContext and
-        not maybeNullArg(cc.getArgument(3))
-      )
-    )
-  )
-}
-
 /**
  * Taint configuration tracking flow from variables containing sensitive information to broadcast Intents.
  */
@@ -165,11 +140,7 @@ class SensitiveCommunicationConfig extends TaintTracking::Configuration {
   /**
    * Holds if broadcast doesn't specify receiving package name of the 3rd party app
    */
-  override predicate isSanitizer(DataFlow::Node node) {
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof ExplicitIntentSanitizer }
-    exists(DataFlow::Node intent | isCleanIntent(intent.asExpr()) |
-      DataFlow::localFlow(intent, node)
-    )
-  }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
     super.allowImplicitRead(node, c)

java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll

@@ -48,11 +48,7 @@ class GetContentIntentConfig extends TaintTracking2::Configuration {
     // Allow the wrapped intent created by Intent.getChooser to be consumed
     // by at the sink:
     isSink(node) and
-    (
+    allowIntentExtrasImplicitRead(node, content)
-      content.(DataFlow::SyntheticFieldContent).getField() = "android.content.Intent.extras"
-      or
-      content instanceof DataFlow::MapValueContent
-    )
   }
 }