Merge pull request #10890 from atorralba/atorralba/android-startactivities-summaries

Java: Add flow summaries for startActivities

Author: atorralba

java/ql/lib/change-notes/2022-10-19-android-startactivities-summaries.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added data flow summaries for tainted Android intents sent to activities via `Activity.startActivities`.

java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll

@@ -26,6 +26,9 @@ module SummaryComponent {
   /** Gets a summary component for `Element`. */
   SummaryComponent element() { result = content(any(CollectionContent c)) }
 
+  /** Gets a summary component for `ArrayElement`. */
+  SummaryComponent arrayElement() { result = content(any(ArrayContent c)) }
+
   /** Gets a summary component for `MapValue`. */
   SummaryComponent mapValue() { result = content(any(MapValueContent c)) }
 
@@ -52,6 +55,11 @@ module SummaryComponentStack {
     result = push(SummaryComponent::element(), object)
   }
 
+  /** Gets a stack representing `ArrayElement` of `object`. */
+  SummaryComponentStack arrayElementOf(SummaryComponentStack object) {
+    result = push(SummaryComponent::arrayElement(), object)
+  }
+
   /** Gets a stack representing `MapValue` of `object`. */
   SummaryComponentStack mapValueOf(SummaryComponentStack object) {
     result = push(SummaryComponent::mapValue(), object)

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

@@ -1,7 +1,10 @@
 import java
+private import semmle.code.java.frameworks.android.Android
 private import semmle.code.java.dataflow.DataFlow
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSteps
+private import semmle.code.java.dataflow.FlowSummary
+private import semmle.code.java.dataflow.internal.BaseSSA as BaseSsa
 
 /** The class `android.content.Intent`. */
 class TypeIntent extends Class {
@@ -242,19 +245,52 @@ private class StartComponentMethodAccess extends MethodAccess {
 
   /** Gets the intent argument of this call. */
   Argument getIntentArg() {
-    result.getType() instanceof TypeIntent and
+    (
+      result.getType() instanceof TypeIntent or
+      result.getType().(Array).getElementType() instanceof TypeIntent
+    ) and
     result = this.getAnArgument()
   }
 
   /** Holds if this targets a component of type `targetType`. */
-  predicate targetsComponentType(RefType targetType) {
+  predicate targetsComponentType(AndroidComponent targetType) {
     exists(NewIntent newIntent |
-      DataFlow::localExprFlow(newIntent, this.getIntentArg()) and
+      reaches(newIntent, this.getIntentArg()) and
       newIntent.getClassArg().getType().(ParameterizedType).getATypeArgument() = targetType
     )
   }
 }
 
+/**
+ * Holds if `src` reaches the intent argument `arg` of `StartComponentMethodAccess`
+ * through intra-procedural steps.
+ */
+private predicate reaches(Expr src, Argument arg) {
+  any(StartComponentMethodAccess ma).getIntentArg() = arg and
+  src = arg
+  or
+  exists(Expr mid, BaseSsa::BaseSsaVariable ssa, BaseSsa::BaseSsaUpdate upd |
+    reaches(mid, arg) and
+    mid = ssa.getAUse() and
+    upd = ssa.getAnUltimateLocalDefinition() and
+    src = upd.getDefiningExpr().(VariableAssign).getSource()
+  )
+  or
+  exists(CastingExpr e | e.getExpr() = src | reaches(e, arg))
+  or
+  exists(ChooseExpr e | e.getAResultExpr() = src | reaches(e, arg))
+  or
+  exists(AssignExpr e | e.getSource() = src | reaches(e, arg))
+  or
+  exists(ArrayCreationExpr e | e.getInit().getAnInit() = src | reaches(e, arg))
+  or
+  exists(StmtExpr e | e.getResultExpr() = src | reaches(e, arg))
+  or
+  exists(NotNullExpr e | e.getExpr() = src | reaches(e, arg))
+  or
+  exists(WhenExpr e | e.getBranch(_).getAResult() = src | reaches(e, arg))
+}
+
 /**
  * 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.
@@ -271,6 +307,87 @@ private class StartActivityIntentStep extends AdditionalValueStep {
   }
 }
 
+/**
+ * Holds if `targetType` is targeted by an existing `StartComponentMethodAccess` call
+ * and it's identified by `id`.
+ */
+private predicate isTargetableType(AndroidComponent targetType, string id) {
+  exists(StartComponentMethodAccess ma | ma.targetsComponentType(targetType)) and
+  targetType.getQualifiedName() = id
+}
+
+private class StartActivitiesSyntheticCallable extends SyntheticCallable {
+  AndroidComponent targetType;
+
+  StartActivitiesSyntheticCallable() {
+    exists(string id |
+      this = "android.content.Activity.startActivities()+" + id and
+      isTargetableType(targetType, id)
+    )
+  }
+
+  override StartComponentMethodAccess getACall() {
+    result.getMethod().hasName("startActivities") and
+    result.targetsComponentType(targetType)
+  }
+
+  override predicate propagatesFlow(
+    SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
+  ) {
+    exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
+      input = SummaryComponentStack::arrayElementOf(SummaryComponentStack::argument(0)) and
+      output = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
+      preservesValue = true
+    )
+  }
+}
+
+private class GetIntentSyntheticCallable extends SyntheticCallable {
+  AndroidComponent targetType;
+
+  GetIntentSyntheticCallable() {
+    exists(string id |
+      this = "android.content.Activity.getIntent()+" + id and
+      isTargetableType(targetType, id)
+    )
+  }
+
+  override Call getACall() {
+    result.getCallee() instanceof AndroidGetIntentMethod and
+    result.getEnclosingCallable().getDeclaringType() = targetType
+  }
+
+  override predicate propagatesFlow(
+    SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
+  ) {
+    exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
+      input = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
+      output = SummaryComponentStack::return() and
+      preservesValue = true
+    )
+  }
+}
+
+private class ActivityIntentSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
+  AndroidComponent targetType;
+
+  ActivityIntentSyntheticGlobal() {
+    exists(string id |
+      this = "ActivityIntentSyntheticGlobal+" + id and
+      isTargetableType(targetType, id)
+    )
+  }
+
+  AndroidComponent getTargetType() { result = targetType }
+}
+
+private class RequiredComponentStackForStartActivities extends RequiredSummaryComponentStack {
+  override predicate required(SummaryComponent head, SummaryComponentStack tail) {
+    head = SummaryComponent::arrayElement() and
+    tail = SummaryComponentStack::argument(0)
+  }
+}
+
 /**
  * A value-preserving step from the intent argument of a `sendBroadcast` call to
  * the intent parameter in the `onReceive` method of the receiver the

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

@@ -24,6 +24,12 @@ public void test(Context ctx, Activity act) {
             Intent[] intents = new Intent[] {intent};
             ctx.startActivities(intents);
         }
+        {
+            Intent intent = new Intent(null, AnotherActivity.class);
+            intent.putExtra("data", (String) source("ctx-start-acts-2"));
+            Intent[] intents = new Intent[] {intent};
+            ctx.startActivities(intents);
+        }
         {
             Intent intent = new Intent(null, SomeActivity.class);
             intent.putExtra("data", (String) source("act-start"));
@@ -35,6 +41,12 @@ public void test(Context ctx, Activity act) {
             Intent[] intents = new Intent[] {intent};
             act.startActivities(intents);
         }
+        {
+            Intent intent = new Intent(null, Object.class);
+            intent.putExtra("data", (String) source("start-activities-should-not-reach"));
+            Intent[] intents = new Intent[] {intent};
+            act.startActivities(intents);
+        }
         {
             Intent intent = new Intent(null, SomeActivity.class);
             intent.putExtra("data", (String) source("start-for-result"));
@@ -79,9 +91,16 @@ public void test(Context ctx, Activity act) {
     static class SomeActivity extends Activity {
 
         public void test() {
-            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
+            // @formatter:off
+            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 hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts
+            // @formatter:on
         }
+    }
 
+    static class AnotherActivity extends Activity {
+        public void test() {
+            sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start-acts-2
+        }
     }
 
     static class SafeActivity extends Activity {