Add support for PendingIntents in Notifications

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

@@ -698,7 +698,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/security/ImplicitPendingIntents.qll

@@ -8,9 +8,13 @@ private class PendingIntentCreationModels extends SinkModelCsv {
     row =
       [
         "android.app;PendingIntent;false;getActivity;;;Argument[2];pending-intent",
+        "android.app;PendingIntent;false;getActivityAsUser;;;Argument[2];pending-intent",
         "android.app;PendingIntent;false;getActivities;;;Argument[2];pending-intent",
+        "android.app;PendingIntent;false;getActivitiesAsUser;;;Argument[2];pending-intent",
         "android.app;PendingIntent;false;getBroadcast;;;Argument[2];pending-intent",
-        "android.app;PendingIntent;false;getService;;;Argument[2];pending-intent"
+        "android.app;PendingIntent;false;getBroadcastAsUser;;;Argument[2];pending-intent",
+        "android.app;PendingIntent;false;getService;;;Argument[2];pending-intent",
+        "android.app;PendingIntent;false;getForegroundService;;;Argument[2];pending-intent"
       ]
   }
 }
@@ -20,7 +24,11 @@ private class PendingIntentSentSinkModels extends SinkModelCsv {
     row =
       [
         "androidx.slice;SliceProvider;true;onBindSlice;;;ReturnValue;pending-intent-sent",
-        "androidx.slice;SliceProvider;true;onCreatePermissionRequest;;;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"
       ]
   }
 }
@@ -53,3 +61,53 @@ private class DefaultIntentRedirectionSinkModel extends SinkModelCsv {
       ]
   }
 }
+
+// TODO: Remove when https://github.com/github/codeql/pull/6823 gets merged
+private class NotificationBuildersSummaryModels extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "android.app;Notification$Action;true;Action;(int,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
+        "android.app;Notification$Action$Builder;true;Builder;(int,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
+        "android.app;Notification$Action$Builder;true;Builder;(Icon,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
+        "android.app;Notification$Action$Builder;true;Builder;(Action);;Argument[0];Argument[-1];taint",
+        "android.app;Notification$Action$Builder;true;addExtras;;;MapKey of Argument[0];MapKey of SyntheticField[android.app.NotificationActionBuilder.extras] of Argument[-1];value",
+        "android.app;Notification$Action$Builder;true;addExtras;;;MapValue of Argument[0];MapValue of SyntheticField[android.app.NotificationActionBuilder.extras] of Argument[-1];value",
+        "android.app;Notification$Action$Builder;true;build;;;Argument[-1];ReturnValue;taint",
+        "android.app;Notification$Action$Builder;true;getExtras;;;SyntheticField[android.app.NotificationActionBuilder.extras] of Argument[-1];ReturnValue;value",
+        "android.app;Notification$Builder;true;addAction;(int,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
+        "android.app;Notification$Builder;true;addAction;(Action);;Argument[0];Argument[-1];taint",
+        "android.app;Notification$Builder;true;addExtras;;;MapKey of Argument[0];MapKey of SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];value",
+        "android.app;Notification$Builder;true;addExtras;;;MapValue of Argument[0];MapValue of SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];value",
+        "android.app;Notification$Builder;true;build;;;Argument[-1];ReturnValue;taint",
+        "android.app;Notification$Builder;true;setContentIntent;;;Argument[0];Argument[-1];taint",
+        "android.app;Notification$Builder;true;getExtras;;;SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];ReturnValue;value",
+        "android.app;Notification$Builder;true;recoverBuilder;;;Argument[1];ReturnValue;taint",
+        "android.app;Notification$Builder;true;setActions;;;ArrayElement of Argument[0];Argument[-1];taint",
+        "android.app;Notification$Builder;true;setExtras;;;Argument[0];SyntheticField[android.app.NotificationBuilder.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",
+        // Fluent models
+        "android.app;Notification$Action$Builder;true;" +
+          [
+            "addExtras", "addRemoteInput", "extend", "setAllowGeneratedReplies",
+            "setAuthenticationRequired", "setContextual", "setSemanticAction"
+          ] + ";;;Argument[-1];ReturnValue;value",
+        "android.app;Notification$Builder;true;" +
+          [
+            "addAction", "addExtras", "addPerson", "extend", "setActions", "setAutoCancel",
+            "setBadgeIconType", "setBubbleMetadata", "setCategory", "setChannelId",
+            "setChronometerCountDown", "setColor", "setColorized", "setContent", "setContentInfo",
+            "setContentIntent", "setContentText", "setContentTitle", "setCustomBigContentView",
+            "setCustomHeadsUpContentView", "setDefaults", "setDeleteIntent", "setExtras", "setFlag",
+            "setForegroundServiceBehavior", "setFullScreenIntent", "setGroup",
+            "setGroupAlertBehavior", "setGroupSummary", "setLargeIcon", "setLights", "setLocalOnly",
+            "setLocusId", "setNumber", "setOngoing", "setOnlyAlertOnce", "setPriority",
+            "setProgress", "setPublicVersion", "setRemoteInputHistory", "setSettingsText",
+            "setShortcutId", "setShowWhen", "setSmallIcon", "setSortKey", "setSound", "setStyle",
+            "setSubText", "setTicker", "setTimeoutAfter", "setUsesChronometer", "setVibrate",
+            "setVisibility", "setWhen"
+          ] + ";;;Argument[-1];ReturnValue;value"
+      ]
+  }
+}

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

@@ -33,9 +33,13 @@ class ImplicitPendingIntentStartConf extends TaintTracking::Configuration {
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
-    super.allowImplicitRead(node, c) or
-    this.isSink(node) or
-    this.isAdditionalTaintStep(node, _)
+    super.allowImplicitRead(node, c)
+    or
+    this.isSink(node) and
+    allowIntentExtrasImplicitRead(node, c)
+    or
+    this.isAdditionalTaintStep(node, _) and
+    c.(DataFlow::FieldContent).getType() instanceof PendingIntent
   }
 }
 
@@ -78,6 +82,13 @@ private class ImplicitPendingIntentConf extends DataFlow2::Configuration {
   override predicate isBarrier(DataFlow::Node barrier) {
     barrier instanceof ExplicitIntentSanitizer
   }
+
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
+    // Allow implicit reads of Intent arrays for sinks like getStartActivities
+    isSink(node) and
+    node.getType().(Array).getElementType() instanceof TypeIntent and
+    c instanceof DataFlow::ArrayContent
+  }
 }
 
 private class PendingIntentSink extends DataFlow::Node {
@@ -86,24 +97,29 @@ private class PendingIntentSink extends DataFlow::Node {
 
 private class MutablePendingIntentSink extends PendingIntentSink {
   MutablePendingIntentSink() {
-    exists(Argument flagArgument |
-      flagArgument = this.asExpr().(Argument).getCall().getArgument(3)
-    |
+    exists(Argument flagArg | flagArg = this.asExpr().(Argument).getCall().getArgument(3) |
       // API < 31, PendingIntents are mutable by default
-      not TaintTracking::localExprTaint(getPendingIntentFlagAccess("FLAG_IMMUTABLE"), flagArgument)
+      not TaintTracking::localExprTaint(any(ImmutablePendingIntentFlag flag).getAnAccess(), flagArg)
       or
       // API >= 31, PendingIntents need to explicitly set mutability
-      TaintTracking::localExprTaint(getPendingIntentFlagAccess("FLAG_MUTABLE"), flagArgument)
+      TaintTracking::localExprTaint(any(MutablePendingIntentFlag flag).getAnAccess(), flagArg)
     )
   }
 }
 
-private Expr getPendingIntentFlagAccess(string flagName) {
-  exists(Field f |
-    f.getDeclaringType() instanceof PendingIntent and
-    f.isPublic() and
-    f.isFinal() and
-    f.hasName(flagName) and
-    f.getAnAccess() = result
-  )
+private class PendingIntentFlag extends Field {
+  PendingIntentFlag() {
+    this.getDeclaringType() instanceof PendingIntent and
+    this.isPublic() and
+    this.isFinal() and
+    this.getName().matches("FLAG_%")
+  }
+}
+
+private class ImmutablePendingIntentFlag extends PendingIntentFlag {
+  ImmutablePendingIntentFlag() { this.hasName("FLAG_IMMUTABLE") }
+}
+
+private class MutablePendingIntentFlag extends PendingIntentFlag {
+  MutablePendingIntentFlag() { this.hasName("FLAG_MUTABLE") }
 }

java/ql/test/query-tests/security/CWE-927/ImplicitPendingIntentsTest.java

@@ -2,7 +2,10 @@ package com.example.test;
 
 import java.io.FileNotFoundException;
 import android.app.Activity;
+import android.app.Notification;
+import android.app.NotificationManager;
 import android.app.PendingIntent;
+import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.content.res.AssetFileDescriptor;
@@ -19,7 +22,8 @@ import androidx.slice.core.SliceHints.ImageMode;
 
 public class ImplicitPendingIntentsTest {
 
-    public static void test(Context ctx) throws PendingIntent.CanceledException {
+    public static void testPendingIntentAsAnExtra(Context ctx)
+            throws PendingIntent.CanceledException {
         {
             Intent baseIntent = new Intent();
             PendingIntent pi = PendingIntent.getActivity(ctx, 0, baseIntent, 0);
@@ -34,6 +38,63 @@ public class ImplicitPendingIntentsTest {
             ctx.startActivity(fwdIntent); // Safe
         }
 
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getActivityAsUser(ctx, 0, baseIntent, 0, null, null);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.startActivity(fwdIntent); // $hasTaintFlow
+        }
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getActivities(ctx, 0, new Intent[] {baseIntent}, 0);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.startActivity(fwdIntent); // $hasTaintFlow
+        }
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getActivitiesAsUser(ctx, 0, new Intent[] {baseIntent},
+                    0, null, null);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.startActivity(fwdIntent); // $hasTaintFlow
+        }
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getBroadcast(ctx, 0, baseIntent, 0);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.sendBroadcast(fwdIntent); // $hasTaintFlow
+        }
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getBroadcastAsUser(ctx, 0, baseIntent, 0, null);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.sendBroadcast(fwdIntent); // $hasTaintFlow
+        }
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getService(ctx, 0, baseIntent, 0);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.startActivity(fwdIntent); // $hasTaintFlow
+        }
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getForegroundService(ctx, 0, baseIntent, 0);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi);
+            ctx.startActivity(fwdIntent); // $hasTaintFlow
+        }
+
         {
             Intent safeIntent = new Intent(ctx, Activity.class); // Sanitizer
             PendingIntent pi = PendingIntent.getActivity(ctx, 0, safeIntent, 0);
@@ -85,6 +146,51 @@ public class ImplicitPendingIntentsTest {
             fwdIntent.putExtra("fwdIntent", pi);
             ctx.startActivity(fwdIntent); // $ SPURIOUS: $ hasTaintFlow
         }
+    }
+
+    public static void testPendingIntentInANotification(Context ctx)
+            throws PendingIntent.CanceledException {
+
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi = PendingIntent.getActivity(ctx, 0, baseIntent, 0);
+            Notification.Action.Builder aBuilder = new Notification.Action.Builder(0, "", pi);
+            Notification.Builder nBuilder =
+                    new Notification.Builder(ctx).addAction(aBuilder.build());
+            Notification notification = nBuilder.build();
+            NotificationManager nManager = new NotificationManager();
+            nManager.notifyAsPackage("targetPackage", "tag", 0, notification); // $hasTaintFlow
+            nManager.notify(0, notification); // $hasTaintFlow
+            nManager.notifyAsUser("", 0, notification, null); // $hasTaintFlow
+        }
+        {
+            Intent baseIntent = new Intent();
+            PendingIntent pi =
+                    PendingIntent.getActivity(ctx, 0, baseIntent, PendingIntent.FLAG_IMMUTABLE); // Sanitizer
+            Notification.Action.Builder aBuilder = new Notification.Action.Builder(0, "", pi);
+            Notification.Builder nBuilder =
+                    new Notification.Builder(ctx).addAction(aBuilder.build());
+            Notification notification = nBuilder.build();
+            NotificationManager nManager = new NotificationManager();
+            nManager.notify(0, notification); // Safe
+        }
+        {
+            // Even though pi1 is vulnerable, it's wrapped in fwdIntent,
+            // from which pi2 (safe) is created. Since only system apps can extract an Intent
+            // from a PendingIntent (via android.permission.GET_INTENT_SENDER_INTENT),
+            // the attacker has no way of accessing fwdIntent, and thus pi1.
+            Intent baseIntent = new Intent();
+            PendingIntent pi1 = PendingIntent.getActivity(ctx, 0, baseIntent, 0);
+            Intent fwdIntent = new Intent();
+            fwdIntent.putExtra("fwdIntent", pi1);
+            PendingIntent pi2 =
+                    PendingIntent.getActivity(ctx, 0, fwdIntent, PendingIntent.FLAG_IMMUTABLE);
+            Notification.Action action = new Notification.Action(0, "", pi2);
+            Notification.Builder nBuilder = new Notification.Builder(ctx).addAction(action);
+            Notification notification = nBuilder.build();
+            NotificationManager noMan = new NotificationManager();
+            noMan.notify(0, notification); // Safe
+        }
 
     }