Merge pull request github#6779 from atorralba/atorralba/android-implicit-pending-intents

Java: CWE-927 - Query to detect the use of implicit PendingIntents

Author: atorralba

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

@@ -118,6 +118,7 @@ private module Frameworks {
   private import semmle.code.java.security.InformationLeak
   private import semmle.code.java.security.Files
   private import semmle.code.java.security.GroovyInjection
+  private import semmle.code.java.security.ImplicitPendingIntents
   private import semmle.code.java.security.JexlInjectionSinkModels
   private import semmle.code.java.security.JndiInjection
   private import semmle.code.java.security.LdapInjection
@@ -685,7 +686,7 @@ class SyntheticField extends string {
 
 private predicate parseSynthField(string c, string f) {
   specSplit(_, c, _) and
-  c.regexpCapture("SyntheticField\\[([.a-zA-Z0-9]+)\\]", 1) = f
+  c.regexpCapture("SyntheticField\\[([.a-zA-Z0-9$]+)\\]", 1) = f
 }
 
 /** Holds if the specification component parses as a `Content`. */

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

@@ -87,13 +87,24 @@ class AndroidBundle extends Class {
   AndroidBundle() { this.getASupertype*().hasQualifiedName("android.os", "BaseBundle") }
 }
 
-/** An `Intent` that explicitly sets a destination component. */
+/**
+ * An `Intent` that explicitly sets a destination component.
+ *
+ * The `Intent` is not considered explicit if a `null` value ever flows to the destination
+ * component, even if only conditionally.
+ *
+ * For example, in the following code, `intent` is not considered an `ExplicitIntent`:
+ * ```java
+ * intent.setClass(condition ? null : "MyClass");
+ * ```
+ */
 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
+      not exists(NullLiteral nullLiteral | DataFlow::localExprFlow(nullLiteral, ma.getAnArgument())) and
       ma.getQualifier() = this
     )
     or

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

@@ -32,6 +32,11 @@ private class NotificationBuildersSummaryModels extends SummaryModelCsv {
         "android.app;Notification$Builder;true;setExtras;;;Argument[0];SyntheticField[android.content.Intent.extras] of Argument[-1];value",
         "android.app;Notification$Builder;true;setDeleteIntent;;;Argument[0];Argument[-1];taint",
         "android.app;Notification$Builder;true;setPublicVersion;;;Argument[0];Argument[-1];taint",
+        "android.app;Notification$Style;true;build;;;Argument[-1];ReturnValue;taint",
+        "android.app;Notification$BigPictureStyle;true;BigPictureStyle;(Builder);;Argument[0];Argument[-1];taint",
+        "android.app;Notification$BigTextStyle;true;BigTextStyle;(Builder);;Argument[0];Argument[-1];taint",
+        "android.app;Notification$InboxStyle;true;InboxStyle;(Builder);;Argument[0];Argument[-1];taint",
+        "android.app;Notification$MediaStyle;true;MediaStyle;(Builder);;Argument[0];Argument[-1];taint",
         // Fluent models
         "android.app;Notification$Action$Builder;true;" +
           [
@@ -52,7 +57,18 @@ private class NotificationBuildersSummaryModels extends SummaryModelCsv {
             "setShortcutId", "setShowWhen", "setSmallIcon", "setSortKey", "setSound", "setStyle",
             "setSubText", "setTicker", "setTimeoutAfter", "setUsesChronometer", "setVibrate",
             "setVisibility", "setWhen"
-          ] + ";;;Argument[-1];ReturnValue;value"
+          ] + ";;;Argument[-1];ReturnValue;value",
+        "android.app;Notification$BigPictureStyle;true;" +
+          [
+            "bigLargeIcon", "bigPicture", "setBigContentTitle", "setContentDescription",
+            "setSummaryText", "showBigPictureWhenCollapsed"
+          ] + ";;;Argument[-1];ReturnValue;value",
+        "android.app;Notification$BigTextStyle;true;" +
+          ["bigText", "setBigContentTitle", "setSummaryText"] + ";;;Argument[-1];ReturnValue;value",
+        "android.app;Notification$InboxStyle;true;" +
+          ["addLine", "setBigContentTitle", "setSummaryText"] + ";;;Argument[-1];ReturnValue;value",
+        "android.app;Notification$MediaStyle;true;" +
+          ["setMediaSession", "setShowActionsInCompactView"] + ";;;Argument[-1];ReturnValue;value"
       ]
   }
 }

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

@@ -0,0 +1,47 @@
+/** Provides classes and predicates related to the class `PendingIntent`. */
+
+import java
+
+/** The class `android.app.PendingIntent`. */
+class PendingIntent extends Class {
+  PendingIntent() { this.hasQualifiedName("android.app", "PendingIntent") }
+}
+
+/** A call to a method that creates a `PendingIntent`. */
+class PendingIntentCreation extends MethodAccess {
+  PendingIntentCreation() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof PendingIntent and
+      m.hasName([
+          "getActivity", "getActivityAsUser", "getActivities", "getActivitiesAsUser",
+          "getBroadcast", "getBroadcastAsUser", "getService", "getForegroundService"
+        ])
+    )
+  }
+
+  /** The `Intent` argument of this call. */
+  Argument getIntentArg() { result = this.getArgument(2) }
+
+  /** The flags argument of this call. */
+  Argument getFlagsArg() { result = this.getArgument(3) }
+}
+
+/** A field of the class `PendingIntent` representing a flag. */
+class PendingIntentFlag extends Field {
+  PendingIntentFlag() {
+    this.getDeclaringType() instanceof PendingIntent and
+    this.isPublic() and
+    this.getName().matches("FLAG_%")
+  }
+}
+
+/** The field `FLAG_IMMUTABLE` of the class `PendingIntent`. */
+class ImmutablePendingIntentFlag extends PendingIntentFlag {
+  ImmutablePendingIntentFlag() { this.hasName("FLAG_IMMUTABLE") }
+}
+
+/** The field `FLAG_MUTABLE` of the class `PendingIntent`. */
+class MutablePendingIntentFlag extends PendingIntentFlag {
+  MutablePendingIntentFlag() { this.hasName("FLAG_MUTABLE") }
+}

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

@@ -0,0 +1,125 @@
+/** Provides classes and predicates for working with implicit `PendingIntent`s. */
+
+import java
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.frameworks.android.Intent
+private import semmle.code.java.frameworks.android.PendingIntent
+
+/** A source for an implicit `PendingIntent` flow. */
+abstract class ImplicitPendingIntentSource extends DataFlow::Node {
+  /** Holds if this source has the specified `state`. */
+  predicate hasState(DataFlow::FlowState state) { state = "" }
+}
+
+/** A sink that sends an implicit and mutable `PendingIntent` to a third party. */
+abstract class ImplicitPendingIntentSink extends DataFlow::Node {
+  /** Holds if this sink has the specified `state`. */
+  predicate hasState(DataFlow::FlowState state) { state = "" }
+}
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to flows related to the use
+ * of implicit `PendingIntent`s.
+ */
+class ImplicitPendingIntentAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for flows related to the use of implicit `PendingIntent`s.
+   */
+  predicate step(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for flows related to the use of implicit `PendingIntent`s. This step is only applicable
+   * in `state1` and updates the flow state to `state2`.
+   */
+  predicate step(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    none()
+  }
+}
+
+private class IntentCreationSource extends ImplicitPendingIntentSource {
+  IntentCreationSource() {
+    exists(ClassInstanceExpr cc |
+      cc.getConstructedType() instanceof TypeIntent and this.asExpr() = cc
+    )
+  }
+}
+
+private class SendPendingIntent extends ImplicitPendingIntentSink {
+  SendPendingIntent() {
+    sinkNode(this, "intent-start") and
+    // implicit intents can't be started as services since API 21
+    not exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      m.getDeclaringType().getASupertype*() instanceof TypeContext and
+      m.getName().matches(["start%Service%", "bindService%"]) and
+      this.asExpr() = ma.getArgument(0)
+    )
+    or
+    sinkNode(this, "pending-intent-sent")
+  }
+
+  override predicate hasState(DataFlow::FlowState state) { state = "MutablePendingIntent" }
+}
+
+/**
+ * Propagates taint from any tainted object to reads from its `PendingIntent`-typed fields.
+ */
+private class PendingIntentAsFieldAdditionalTaintStep extends ImplicitPendingIntentAdditionalTaintStep {
+  override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(Field f |
+      f.getType() instanceof PendingIntent and
+      node1.(DataFlow::PostUpdateNode).getPreUpdateNode() =
+        DataFlow::getFieldQualifier(f.getAnAccess().(FieldWrite)) and
+      node2.asExpr().(FieldRead).getField() = f
+    )
+  }
+}
+
+private class MutablePendingIntentFlowStep extends PendingIntentAsFieldAdditionalTaintStep {
+  override predicate step(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    state1 = "" and
+    state2 = "MutablePendingIntent" and
+    exists(PendingIntentCreation pic, Argument flagArg |
+      node1.asExpr() = pic.getIntentArg() and
+      node2.asExpr() = pic and
+      flagArg = pic.getFlagsArg()
+    |
+      // We err on the side of false positives here, assuming a PendingIntent may be mutable
+      // unless it is at least sometimes explicitly marked immutable and never marked mutable.
+      // Note: for API level < 31, PendingIntents were mutable by default, whereas since then
+      // they are immutable by default.
+      not TaintTracking::localExprTaint(any(ImmutablePendingIntentFlag flag).getAnAccess(), flagArg)
+      or
+      TaintTracking::localExprTaint(any(MutablePendingIntentFlag flag).getAnAccess(), flagArg)
+    )
+  }
+}
+
+private class PendingIntentSentSinkModels extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "androidx.slice;SliceProvider;true;onBindSlice;;;ReturnValue;pending-intent-sent",
+        "androidx.slice;SliceProvider;true;onCreatePermissionRequest;;;ReturnValue;pending-intent-sent",
+        "android.app;NotificationManager;true;notify;(int,Notification);;Argument[1];pending-intent-sent",
+        "android.app;NotificationManager;true;notify;(String,int,Notification);;Argument[2];pending-intent-sent",
+        "android.app;NotificationManager;true;notifyAsPackage;(String,String,int,Notification);;Argument[3];pending-intent-sent",
+        "android.app;NotificationManager;true;notifyAsUser;(String,int,Notification,UserHandle);;Argument[2];pending-intent-sent",
+        "android.app;PendingIntent;false;send;(Context,int,Intent,OnFinished,Handler,String,Bundle);;Argument[2];pending-intent-sent",
+        "android.app;PendingIntent;false;send;(Context,int,Intent,OnFinished,Handler,String);;Argument[2];pending-intent-sent",
+        "android.app;PendingIntent;false;send;(Context,int,Intent,OnFinished,Handler);;Argument[2];pending-intent-sent",
+        "android.app;PendingIntent;false;send;(Context,int,Intent);;Argument[2];pending-intent-sent",
+        "android.app;Activity;true;setResult;(int,Intent);;Argument[1];pending-intent-sent"
+      ]
+  }
+}

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

@@ -0,0 +1,54 @@
+/** Provides taint tracking configurations to be used in queries related to implicit `PendingIntent`s. */
+
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.frameworks.android.Intent
+import semmle.code.java.frameworks.android.PendingIntent
+import semmle.code.java.security.ImplicitPendingIntents
+
+/**
+ * A taint tracking configuration for implicit `PendingIntent`s
+ * being wrapped in another implicit `Intent` that gets started.
+ */
+class ImplicitPendingIntentStartConf extends TaintTracking::Configuration {
+  ImplicitPendingIntentStartConf() { this = "ImplicitPendingIntentStartConf" }
+
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+    source.(ImplicitPendingIntentSource).hasState(state)
+  }
+
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
+    sink.(ImplicitPendingIntentSink).hasState(state)
+  }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof ExplicitIntentSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, node2)
+  }
+
+  override predicate isAdditionalFlowStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, state1, node2, state2)
+  }
+
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
+    super.allowImplicitRead(node, c)
+    or
+    this.isSink(node, _) and
+    allowIntentExtrasImplicitRead(node, c)
+    or
+    this.isAdditionalTaintStep(node, _) and
+    c.(DataFlow::FieldContent).getType() instanceof PendingIntent
+    or
+    // Allow implicit reads of Intent arrays for steps like getActivities
+    // or sinks like startActivities
+    (this.isSink(node, _) or this.isAdditionalFlowStep(node, _, _, _)) and
+    node.getType().(Array).getElementType() instanceof TypeIntent and
+    c instanceof DataFlow::ArrayContent
+  }
+}

java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.java

@@ -0,0 +1,43 @@
+import android.app.Activity;
+import android.app.PendingIntent;
+import android.content.Intent;
+import android.os.Bundle;
+
+public class ImplicitPendingIntents extends Activity {
+
+	public void onCreate(Bundle savedInstance) {
+		{
+			// BAD: an implicit Intent is used to create a PendingIntent.
+			// The PendingIntent is then added to another implicit Intent
+			// and started.
+			Intent baseIntent = new Intent();
+			PendingIntent pi =
+					PendingIntent.getActivity(this, 0, baseIntent, PendingIntent.FLAG_ONE_SHOT);
+			Intent fwdIntent = new Intent("SOME_ACTION");
+			fwdIntent.putExtra("fwdIntent", pi);
+			sendBroadcast(fwdIntent);
+		}
+
+		{
+			// GOOD: both the PendingIntent and the wrapping Intent are explicit.
+			Intent safeIntent = new Intent(this, AnotherActivity.class);
+			PendingIntent pi =
+					PendingIntent.getActivity(this, 0, safeIntent, PendingIntent.FLAG_ONE_SHOT);
+			Intent fwdIntent = new Intent();
+			fwdIntent.setClassName("destination.package", "DestinationClass");
+			fwdIntent.putExtra("fwdIntent", pi);
+			startActivity(fwdIntent);
+		}
+
+		{
+			// GOOD: The PendingIntent is created with FLAG_IMMUTABLE.
+			Intent baseIntent = new Intent("SOME_ACTION");
+			PendingIntent pi =
+					PendingIntent.getActivity(this, 0, baseIntent, PendingIntent.FLAG_IMMUTABLE);
+			Intent fwdIntent = new Intent();
+			fwdIntent.setClassName("destination.package", "DestinationClass");
+			fwdIntent.putExtra("fwdIntent", pi);
+			startActivity(fwdIntent);
+		}
+	}
+}