Merge pull request #10368 from jcogs33/android-deeplink-analysis

Java: Android deeplink analysis

java/ql/lib/change-notes/2022-09-22-android-deeplink-flow-steps.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* Added data flow steps for tainted Android intents that are sent to services and receivers.
+* Improved the data flow step for tainted Android intents that are sent to activities so that more cases are covered.

java/ql/lib/change-notes/2022-09-22-android-deprecate-contextstartactivitymethod.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* Deprecated `ContextStartActivityMethod`. Use `StartActivityMethod` instead.

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

@@ -3,9 +3,7 @@ private import semmle.code.java.dataflow.DataFlow
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSteps
 
-/**
- * The class `android.content.Intent`.
- */
+/** The class `android.content.Intent`. */
 class TypeIntent extends Class {
   TypeIntent() { this.hasQualifiedName("android.content", "Intent") }
 }
@@ -15,23 +13,17 @@ class TypeComponentName extends Class {
   TypeComponentName() { this.hasQualifiedName("android.content", "ComponentName") }
 }
 
-/**
- * The class `android.app.Activity`.
- */
+/** The class `android.app.Activity`. */
 class TypeActivity extends Class {
   TypeActivity() { this.hasQualifiedName("android.app", "Activity") }
 }
 
-/**
- * The class `android.app.Service`.
- */
+/** The class `android.app.Service`. */
 class TypeService extends Class {
   TypeService() { this.hasQualifiedName("android.app", "Service") }
 }
 
-/**
- * The class `android.content.Context`.
- */
+/** The class `android.content.Context`. */
 class TypeContext extends RefType {
   // Not inlining this makes it more likely to be used as a sentinel,
   // which is useful when running Android queries on non-Android projects.
@@ -39,25 +31,19 @@ class TypeContext extends RefType {
   TypeContext() { this.hasQualifiedName("android.content", "Context") }
 }
 
-/**
- * The class `android.content.BroadcastReceiver`.
- */
+/** The class `android.content.BroadcastReceiver`. */
 class TypeBroadcastReceiver extends Class {
   TypeBroadcastReceiver() { this.hasQualifiedName("android.content", "BroadcastReceiver") }
 }
 
-/**
- * The method `Activity.getIntent`
- */
+/** The method `Activity.getIntent` */
 class AndroidGetIntentMethod extends Method {
   AndroidGetIntentMethod() {
     this.hasName("getIntent") and this.getDeclaringType() instanceof TypeActivity
   }
 }
 
-/**
- * The method `BroadcastReceiver.onReceive`.
- */
+/** The method `BroadcastReceiver.onReceive`. */
 class AndroidReceiveIntentMethod extends Method {
   AndroidReceiveIntentMethod() {
     this.hasName("onReceive") and this.getDeclaringType() instanceof TypeBroadcastReceiver
@@ -77,8 +63,10 @@ class AndroidServiceIntentMethod extends Method {
 
 /**
  * The method `Context.startActivity` or `startActivities`.
+ *
+ * DEPRECATED: Use `StartActivityMethod` instead.
  */
-class ContextStartActivityMethod extends Method {
+deprecated class ContextStartActivityMethod extends Method {
   ContextStartActivityMethod() {
     (this.hasName("startActivity") or this.hasName("startActivities")) and
     this.getDeclaringType() instanceof TypeContext
@@ -86,16 +74,57 @@ class ContextStartActivityMethod extends Method {
 }
 
 /**
- * Specifies that if an `Intent` is tainted, then so are its synthetic fields.
+ * The method `Context.startActivity`, `Context.startActivities`,
+ * `Activity.startActivity`,`Activity.startActivities`,
+ * `Activity.startActivityForResult`, `Activity.startActivityIfNeeded`,
+ * `Activity.startNextMatchingActivity`, `Activity.startActivityFromChild`,
+ * or `Activity.startActivityFromFragment`.
  */
+class StartActivityMethod extends Method {
+  StartActivityMethod() {
+    this.getName().matches("start%Activit%") and
+    (
+      this.getDeclaringType() instanceof TypeContext or
+      this.getDeclaringType() instanceof TypeActivity
+    )
+  }
+}
+
+/**
+ * The method `Context.sendBroadcast`, `sendBroadcastAsUser`,
+ * `sendOrderedBroadcast`, `sendOrderedBroadcastAsUser`,
+ * `sendStickyBroadcast`, `sendStickyBroadcastAsUser`,
+ * `sendStickyOrderedBroadcast`, `sendStickyOrderedBroadcastAsUser`,
+ * or `sendBroadcastWithMultiplePermissions`.
+ */
+class SendBroadcastMethod extends Method {
+  SendBroadcastMethod() {
+    this.getName().matches("send%Broadcast%") and
+    this.getDeclaringType() instanceof TypeContext
+  }
+}
+
+/**
+ * The method `Context.startService`, `startForegroundService`,
+ * `bindIsolatedService`, `bindService`, or `bindServiceAsUser`.
+ */
+class StartServiceMethod extends Method {
+  StartServiceMethod() {
+    this.hasName([
+        "startService", "startForegroundService", "bindIsolatedService", "bindService",
+        "bindServiceAsUser"
+      ]) and
+    this.getDeclaringType() instanceof TypeContext
+  }
+}
+
+/** Specifies that if an `Intent` is tainted, then so are its synthetic fields. */
 private class IntentFieldsInheritTaint extends DataFlow::SyntheticFieldContent,
   TaintInheritingContent {
   IntentFieldsInheritTaint() { this.getField().matches("android.content.Intent.%") }
 }
 
-/**
- * The method `Intent.getParcelableExtra`.
- */
+/** The method `Intent.getParcelableExtra`. */
 class IntentGetParcelableExtraMethod extends Method {
   IntentGetParcelableExtraMethod() {
     this.hasName("getParcelableExtra") and
@@ -157,9 +186,7 @@ private class BundleExtrasSyntheticField extends SyntheticField {
   override RefType getType() { result instanceof AndroidBundle }
 }
 
-/**
- * Holds if extras may be implicitly read from the Intent `node`.
- */
+/** Holds if extras may be implicitly read from the Intent `node`. */
 predicate allowIntentExtrasImplicitRead(DataFlow::Node node, DataFlow::Content c) {
   node.getType() instanceof TypeIntent and
   (
@@ -194,25 +221,90 @@ class GrantWriteUriPermissionFlag extends GrantUriPermissionFlag {
   GrantWriteUriPermissionFlag() { this.hasName("FLAG_GRANT_WRITE_URI_PERMISSION") }
 }
 
+/** An instantiation of `android.content.Intent`. */
+private class NewIntent extends ClassInstanceExpr {
+  NewIntent() { this.getConstructedType() instanceof TypeIntent }
+
+  /** Gets the `Class<?>` argument of this call. */
+  Argument getClassArg() {
+    result.getType() instanceof TypeClass and
+    result = this.getAnArgument()
+  }
+}
+
+/** A call to a method that starts an Android component. */
+private class StartComponentMethodAccess extends MethodAccess {
+  StartComponentMethodAccess() {
+    this.getMethod().overrides*(any(StartActivityMethod m)) or
+    this.getMethod().overrides*(any(StartServiceMethod m)) or
+    this.getMethod().overrides*(any(SendBroadcastMethod m))
+  }
+
+  /** Gets the intent argument of this call. */
+  Argument getIntentArg() {
+    result.getType() instanceof TypeIntent and
+    result = this.getAnArgument()
+  }
+
+  /** Holds if this targets a component of type `targetType`. */
+  predicate targetsComponentType(RefType targetType) {
+    exists(NewIntent newIntent |
+      DataFlow::localExprFlow(newIntent, this.getIntentArg()) and
+      newIntent.getClassArg().getType().(ParameterizedType).getATypeArgument() = targetType
+    )
+  }
+}
+
 /**
- * A value-preserving step from the Intent argument of a `startActivity` call to
- * a `getIntent` call in the Activity the Intent pointed to in its constructor.
+ * A value-preserving step from the intent argument of a `startActivity` call to
+ * a `getIntent` call in the activity the intent targeted in its constructor.
  */
 private class StartActivityIntentStep extends AdditionalValueStep {
   override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
-    exists(MethodAccess startActivity, MethodAccess getIntent, ClassInstanceExpr newIntent |
-      startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) and
+    exists(StartComponentMethodAccess startActivity, MethodAccess getIntent |
+      startActivity.getMethod().overrides*(any(StartActivityMethod m)) and
       getIntent.getMethod().overrides*(any(AndroidGetIntentMethod m)) and
-      newIntent.getConstructedType() instanceof TypeIntent and
-      DataFlow::localExprFlow(newIntent, startActivity.getArgument(0)) and
-      newIntent.getArgument(1).getType().(ParameterizedType).getATypeArgument() =
-        getIntent.getReceiverType() and
-      n1.asExpr() = startActivity.getArgument(0) and
+      startActivity.targetsComponentType(getIntent.getReceiverType()) and
+      n1.asExpr() = startActivity.getIntentArg() and
       n2.asExpr() = getIntent
     )
   }
 }
 
+/**
+ * A value-preserving step from the intent argument of a `sendBroadcast` call to
+ * the intent parameter in the `onReceive` method of the receiver the
+ * intent targeted in its constructor.
+ */
+private class SendBroadcastReceiverIntentStep extends AdditionalValueStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(StartComponentMethodAccess sendBroadcast, Method onReceive |
+      sendBroadcast.getMethod().overrides*(any(SendBroadcastMethod m)) and
+      onReceive.overrides*(any(AndroidReceiveIntentMethod m)) and
+      sendBroadcast.targetsComponentType(onReceive.getDeclaringType()) and
+      n1.asExpr() = sendBroadcast.getIntentArg() and
+      n2.asParameter() = onReceive.getParameter(1)
+    )
+  }
+}
+
+/**
+ * A value-preserving step from the intent argument of a `startService` call to
+ * the intent parameter in an `AndroidServiceIntentMethod` of the service the
+ * intent targeted in its constructor.
+ */
+private class StartServiceIntentStep extends AdditionalValueStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(StartComponentMethodAccess startService, Method serviceIntent |
+      startService.getMethod().overrides*(any(StartServiceMethod m)) and
+      serviceIntent.overrides*(any(AndroidServiceIntentMethod m)) and
+      startService.targetsComponentType(serviceIntent.getDeclaringType()) and
+      n1.asExpr() = startService.getIntentArg() and
+      n2.asParameter() = serviceIntent.getParameter(0)
+    )
+  }
+}
+
 private class IntentBundleFlowSteps extends SummaryModelCsv {
   override predicate row(string row) {
     row =

java/ql/test/library-tests/frameworks/android/intent/AndroidManifest.xml

@@ -18,5 +18,30 @@
             android:exported="false">
         </activity>
 
+        <activity
+            android:name=".TestStartActivityToGetIntent.SafeActivity"
+            android:exported="false">
+        </activity>
+
+        <service
+            android:name=".TestStartServiceToGetIntent.SomeService"
+            android:exported="false">
+        </service>
+
+        <service
+            android:name=".TestStartServiceToGetIntent.SafeService"
+            android:exported="false">
+        </service>
+
+        <receiver
+            android:name=".TestStartBroadcastReceiverToGetIntent.SomeBroadcastReceiver"
+            android:exported="false">
+        </receiver>
+
+        <receiver
+            android:name=".TestStartBroadcastReceiverToGetIntent.SafeBroadcastReceiver"
+            android:exported="false">
+        </receiver>
+
     </application>
 </manifest>

java/ql/test/library-tests/frameworks/android/intent/TestStartActivityToGetIntent.java

@@ -4,22 +4,90 @@ import android.content.Intent;
 
 public class TestStartActivityToGetIntent {
 
-    static Object source() {
+    static Object source(String kind) {
         return null;
     }
 
     static void sink(Object sink) {}
 
-    public void test(Context ctx) {
-        Intent intent = new Intent(null, SomeActivity.class);
-        intent.putExtra("data", (String) source());
-        ctx.startActivity(intent);
+    public void test(Context ctx, Activity act) {
+
+        // test all methods that start an activity
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("ctx-start"));
+            ctx.startActivity(intent);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("ctx-start-acts"));
+            Intent[] intents = new Intent[] {intent};
+            ctx.startActivities(intents);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("act-start"));
+            act.startActivity(intent);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("act-start-acts"));
+            Intent[] intents = new Intent[] {intent};
+            act.startActivities(intents);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("start-for-result"));
+            act.startActivityForResult(intent, 0);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("start-if-needed"));
+            act.startActivityIfNeeded(intent, 0);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("start-matching"));
+            act.startNextMatchingActivity(intent);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("start-from-child"));
+            act.startActivityFromChild(null, intent, 0);
+        }
+        {
+            Intent intent = new Intent(null, SomeActivity.class);
+            intent.putExtra("data", (String) source("start-from-frag"));
+            act.startActivityFromFragment(null, intent, 0);
+        }
+
+        // test 4-arg Intent constructor
+        {
+            Intent intent = new Intent(null, null, null, SomeActivity.class);
+            intent.putExtra("data", (String) source("4-arg"));
+            ctx.startActivity(intent);
+        }
+
+        // safe test
+        {
+            Intent intent = new Intent(null, SafeActivity.class);
+            intent.putExtra("data", "safe");
+            ctx.startActivity(intent);
+        }
     }
 
     static class SomeActivity extends Activity {
 
         public void test() {
-            sink(getIntent().getStringExtra("data")); // $ hasValueFlow
+            sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start hasValueFlow=act-start hasValueFlow=start-for-result hasValueFlow=start-if-needed hasValueFlow=start-matching hasValueFlow=start-from-child hasValueFlow=start-from-frag hasValueFlow=4-arg MISSING: hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts
+        }
+
+    }
+
+    static class SafeActivity extends Activity {
+
+        public void test() {
+            sink(getIntent().getStringExtra("data")); // Safe
         }
     }
 }

java/ql/test/library-tests/frameworks/android/intent/TestStartBroadcastReceiverToIntent.java

@@ -0,0 +1,93 @@
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.Intent;
+
+public class TestStartBroadcastReceiverToIntent {
+
+    static Object source(String kind) {
+        return null;
+    }
+
+    static void sink(Object sink) {}
+
+    public void test(Context ctx) {
+
+        // test all methods that send a broadcast
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send"));
+            ctx.sendBroadcast(intent);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-as-user"));
+            ctx.sendBroadcastAsUser(intent, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-with-perm"));
+            ctx.sendBroadcastWithMultiplePermissions(intent, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-ordered"));
+            ctx.sendOrderedBroadcast(intent, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-ordered-as-user"));
+            ctx.sendOrderedBroadcastAsUser(intent, null, null, null, null, 0, null, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-sticky"));
+            ctx.sendStickyBroadcast(intent);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-sticky-as-user"));
+            ctx.sendStickyBroadcastAsUser(intent, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-sticky-ordered"));
+            ctx.sendStickyOrderedBroadcast(intent, null, null, 0, null, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("send-sticky-ordered-as-user"));
+            ctx.sendStickyOrderedBroadcastAsUser(intent, null, null, null, 0, null, null);
+        }
+
+        // test 4-arg Intent constructor
+        {
+            Intent intent = new Intent(null, null, null, SomeBroadcastReceiver.class);
+            intent.putExtra("data", (String) source("4-arg"));
+            ctx.sendBroadcast(intent);
+        }
+
+        // safe test
+        {
+            Intent intent = new Intent(null, SafeBroadcastReceiver.class);
+            intent.putExtra("data", "safe");
+            ctx.sendBroadcast(intent);
+        }
+    }
+
+    static class SomeBroadcastReceiver extends BroadcastReceiver {
+
+        // test method that receives an Intent as a parameter
+        @Override
+        public void onReceive(Context context, Intent intent) {
+            sink(intent.getStringExtra("data")); // $  hasValueFlow=send hasValueFlow=send-as-user hasValueFlow=send-with-perm hasValueFlow=send-ordered hasValueFlow=send-ordered-as-user hasValueFlow=send-sticky hasValueFlow=send-sticky-as-user hasValueFlow=send-sticky-ordered hasValueFlow=send-sticky-ordered-as-user hasValueFlow=4-arg
+        }
+    }
+
+    static class SafeBroadcastReceiver extends BroadcastReceiver {
+
+        @Override
+        public void onReceive(Context context, Intent intent) {
+            sink(intent.getStringExtra("data")); // Safe
+        }
+    }
+}

java/ql/test/library-tests/frameworks/android/intent/TestStartServiceToIntent.java

@@ -0,0 +1,130 @@
+import android.app.Service;
+import android.os.IBinder;
+import android.content.Context;
+import android.content.Intent;
+
+public class TestStartServiceToIntent {
+
+    static Object source(String kind) {
+        return null;
+    }
+
+    static void sink(Object sink) {}
+
+    public void test(Context ctx) {
+
+        // test all methods that start a service
+        {
+            Intent intent = new Intent(null, SomeService.class);
+            intent.putExtra("data", (String) source("bind"));
+            ctx.bindService(intent, null, 0);
+        }
+        {
+            Intent intent = new Intent(null, SomeService.class);
+            intent.putExtra("data", (String) source("bind-as-user"));
+            ctx.bindServiceAsUser(intent, null, 0, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeService.class);
+            intent.putExtra("data", (String) source("bind-isolated"));
+            ctx.bindIsolatedService(intent, 0, null, null, null);
+        }
+        {
+            Intent intent = new Intent(null, SomeService.class);
+            intent.putExtra("data", (String) source("start"));
+            ctx.startService(intent);
+        }
+        {
+            Intent intent = new Intent(null, SomeService.class);
+            intent.putExtra("data", (String) source("start-foreground"));
+            ctx.startForegroundService(intent);
+        }
+
+        // test 4-arg Intent constructor
+        {
+            Intent intent = new Intent(null, null, null, SomeService.class);
+            intent.putExtra("data", (String) source("4-arg"));
+            ctx.startService(intent);
+        }
+
+        // safe test
+        {
+            Intent intent = new Intent(null, SafeService.class);
+            intent.putExtra("data", "safe");
+            ctx.startService(intent);
+        }
+    }
+
+    static class SomeService extends Service {
+
+        // test methods that receive an Intent as a parameter
+        @Override
+        public void onStart(Intent intent, int startId) {
+            sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg
+        }
+
+        @Override
+        public int onStartCommand(Intent intent, int flags, int startId) {
+            sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg
+            return -1;
+        }
+
+        @Override
+        public IBinder onBind(Intent intent) {
+            sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg
+            return null;
+        }
+
+        @Override
+        public boolean onUnbind(Intent intent) {
+            sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg
+            return false;
+        }
+
+        @Override
+        public void onRebind(Intent intent) {
+            sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg
+        }
+
+        @Override
+        public void onTaskRemoved(Intent intent) {
+            sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg
+        }
+    }
+
+    static class SafeService extends Service {
+
+        @Override
+        public void onStart(Intent intent, int startId) {
+            sink(intent.getStringExtra("data")); // Safe
+        }
+
+        @Override
+        public int onStartCommand(Intent intent, int flags, int startId) {
+            sink(intent.getStringExtra("data")); // Safe
+            return -1;
+        }
+
+        @Override
+        public IBinder onBind(Intent intent) {
+            sink(intent.getStringExtra("data")); // Safe
+            return null;
+        }
+
+        @Override
+        public boolean onUnbind(Intent intent) {
+            sink(intent.getStringExtra("data")); // Safe
+            return false;
+        }
+
+        @Override
+        public void onRebind(Intent intent) {
+            sink(intent.getStringExtra("data")); // Safe
+        }
+
+        @Override
+        public void onTaskRemoved(Intent intent) {
+            sink(intent.getStringExtra("data")); // Safe
+        }
+    }
+}