Skip to content

Commit d49e52f

Browse files
atorralbaatorralba
committed
Add support for PendingIntents in Notifications
1 parent c73e4eb commit d49e52f

File tree

4 files changed

+200
-20
lines changed

4 files changed

+200
-20
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ class SyntheticField extends string {
698698

699699
private predicate parseSynthField(string c, string f) {
700700
specSplit(_, c, _) and
701-
c.regexpCapture("SyntheticField\\[([.a-zA-Z0-9]+)\\]", 1) = f
701+
c.regexpCapture("SyntheticField\\[([.a-zA-Z0-9$]+)\\]", 1) = f
702702
}
703703

704704
/** Holds if the specification component parses as a `Content`. */

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

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@ private class PendingIntentCreationModels extends SinkModelCsv {
88
row =
99
[
1010
"android.app;PendingIntent;false;getActivity;;;Argument[2];pending-intent",
11+
"android.app;PendingIntent;false;getActivityAsUser;;;Argument[2];pending-intent",
1112
"android.app;PendingIntent;false;getActivities;;;Argument[2];pending-intent",
13+
"android.app;PendingIntent;false;getActivitiesAsUser;;;Argument[2];pending-intent",
1214
"android.app;PendingIntent;false;getBroadcast;;;Argument[2];pending-intent",
13-
"android.app;PendingIntent;false;getService;;;Argument[2];pending-intent"
15+
"android.app;PendingIntent;false;getBroadcastAsUser;;;Argument[2];pending-intent",
16+
"android.app;PendingIntent;false;getService;;;Argument[2];pending-intent",
17+
"android.app;PendingIntent;false;getForegroundService;;;Argument[2];pending-intent"
1418
]
1519
}
1620
}
@@ -20,7 +24,11 @@ private class PendingIntentSentSinkModels extends SinkModelCsv {
2024
row =
2125
[
2226
"androidx.slice;SliceProvider;true;onBindSlice;;;ReturnValue;pending-intent-sent",
23-
"androidx.slice;SliceProvider;true;onCreatePermissionRequest;;;ReturnValue;pending-intent-sent"
27+
"androidx.slice;SliceProvider;true;onCreatePermissionRequest;;;ReturnValue;pending-intent-sent",
28+
"android.app;NotificationManager;true;notify;(int,Notification);;Argument[1];pending-intent-sent",
29+
"android.app;NotificationManager;true;notify;(String,int,Notification);;Argument[2];pending-intent-sent",
30+
"android.app;NotificationManager;true;notifyAsPackage;(String,String,int,Notification);;Argument[3];pending-intent-sent",
31+
"android.app;NotificationManager;true;notifyAsUser;(String,int,Notification,UserHandle);;Argument[2];pending-intent-sent"
2432
]
2533
}
2634
}
@@ -53,3 +61,53 @@ private class DefaultIntentRedirectionSinkModel extends SinkModelCsv {
5361
]
5462
}
5563
}
64+
65+
// TODO: Remove when https://github.com/github/codeql/pull/6823 gets merged
66+
private class NotificationBuildersSummaryModels extends SummaryModelCsv {
67+
override predicate row(string row) {
68+
row =
69+
[
70+
"android.app;Notification$Action;true;Action;(int,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
71+
"android.app;Notification$Action$Builder;true;Builder;(int,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
72+
"android.app;Notification$Action$Builder;true;Builder;(Icon,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
73+
"android.app;Notification$Action$Builder;true;Builder;(Action);;Argument[0];Argument[-1];taint",
74+
"android.app;Notification$Action$Builder;true;addExtras;;;MapKey of Argument[0];MapKey of SyntheticField[android.app.NotificationActionBuilder.extras] of Argument[-1];value",
75+
"android.app;Notification$Action$Builder;true;addExtras;;;MapValue of Argument[0];MapValue of SyntheticField[android.app.NotificationActionBuilder.extras] of Argument[-1];value",
76+
"android.app;Notification$Action$Builder;true;build;;;Argument[-1];ReturnValue;taint",
77+
"android.app;Notification$Action$Builder;true;getExtras;;;SyntheticField[android.app.NotificationActionBuilder.extras] of Argument[-1];ReturnValue;value",
78+
"android.app;Notification$Builder;true;addAction;(int,CharSequence,PendingIntent);;Argument[2];Argument[-1];taint",
79+
"android.app;Notification$Builder;true;addAction;(Action);;Argument[0];Argument[-1];taint",
80+
"android.app;Notification$Builder;true;addExtras;;;MapKey of Argument[0];MapKey of SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];value",
81+
"android.app;Notification$Builder;true;addExtras;;;MapValue of Argument[0];MapValue of SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];value",
82+
"android.app;Notification$Builder;true;build;;;Argument[-1];ReturnValue;taint",
83+
"android.app;Notification$Builder;true;setContentIntent;;;Argument[0];Argument[-1];taint",
84+
"android.app;Notification$Builder;true;getExtras;;;SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];ReturnValue;value",
85+
"android.app;Notification$Builder;true;recoverBuilder;;;Argument[1];ReturnValue;taint",
86+
"android.app;Notification$Builder;true;setActions;;;ArrayElement of Argument[0];Argument[-1];taint",
87+
"android.app;Notification$Builder;true;setExtras;;;Argument[0];SyntheticField[android.app.NotificationBuilder.extras] of Argument[-1];value",
88+
"android.app;Notification$Builder;true;setDeleteIntent;;;Argument[0];Argument[-1];taint",
89+
"android.app;Notification$Builder;true;setPublicVersion;;;Argument[0];Argument[-1];taint",
90+
// Fluent models
91+
"android.app;Notification$Action$Builder;true;" +
92+
[
93+
"addExtras", "addRemoteInput", "extend", "setAllowGeneratedReplies",
94+
"setAuthenticationRequired", "setContextual", "setSemanticAction"
95+
] + ";;;Argument[-1];ReturnValue;value",
96+
"android.app;Notification$Builder;true;" +
97+
[
98+
"addAction", "addExtras", "addPerson", "extend", "setActions", "setAutoCancel",
99+
"setBadgeIconType", "setBubbleMetadata", "setCategory", "setChannelId",
100+
"setChronometerCountDown", "setColor", "setColorized", "setContent", "setContentInfo",
101+
"setContentIntent", "setContentText", "setContentTitle", "setCustomBigContentView",
102+
"setCustomHeadsUpContentView", "setDefaults", "setDeleteIntent", "setExtras", "setFlag",
103+
"setForegroundServiceBehavior", "setFullScreenIntent", "setGroup",
104+
"setGroupAlertBehavior", "setGroupSummary", "setLargeIcon", "setLights", "setLocalOnly",
105+
"setLocusId", "setNumber", "setOngoing", "setOnlyAlertOnce", "setPriority",
106+
"setProgress", "setPublicVersion", "setRemoteInputHistory", "setSettingsText",
107+
"setShortcutId", "setShowWhen", "setSmallIcon", "setSortKey", "setSound", "setStyle",
108+
"setSubText", "setTicker", "setTimeoutAfter", "setUsesChronometer", "setVibrate",
109+
"setVisibility", "setWhen"
110+
] + ";;;Argument[-1];ReturnValue;value"
111+
]
112+
}
113+
}

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

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,13 @@ class ImplicitPendingIntentStartConf extends TaintTracking::Configuration {
3333
}
3434

3535
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
36-
super.allowImplicitRead(node, c) or
37-
this.isSink(node) or
38-
this.isAdditionalTaintStep(node, _)
36+
super.allowImplicitRead(node, c)
37+
or
38+
this.isSink(node) and
39+
allowIntentExtrasImplicitRead(node, c)
40+
or
41+
this.isAdditionalTaintStep(node, _) and
42+
c.(DataFlow::FieldContent).getType() instanceof PendingIntent
3943
}
4044
}
4145

@@ -78,6 +82,13 @@ private class ImplicitPendingIntentConf extends DataFlow2::Configuration {
7882
override predicate isBarrier(DataFlow::Node barrier) {
7983
barrier instanceof ExplicitIntentSanitizer
8084
}
85+
86+
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
87+
// Allow implicit reads of Intent arrays for sinks like getStartActivities
88+
isSink(node) and
89+
node.getType().(Array).getElementType() instanceof TypeIntent and
90+
c instanceof DataFlow::ArrayContent
91+
}
8192
}
8293

8394
private class PendingIntentSink extends DataFlow::Node {
@@ -86,24 +97,29 @@ private class PendingIntentSink extends DataFlow::Node {
8697

8798
private class MutablePendingIntentSink extends PendingIntentSink {
8899
MutablePendingIntentSink() {
89-
exists(Argument flagArgument |
90-
flagArgument = this.asExpr().(Argument).getCall().getArgument(3)
91-
|
100+
exists(Argument flagArg | flagArg = this.asExpr().(Argument).getCall().getArgument(3) |
92101
// API < 31, PendingIntents are mutable by default
93-
not TaintTracking::localExprTaint(getPendingIntentFlagAccess("FLAG_IMMUTABLE"), flagArgument)
102+
not TaintTracking::localExprTaint(any(ImmutablePendingIntentFlag flag).getAnAccess(), flagArg)
94103
or
95104
// API >= 31, PendingIntents need to explicitly set mutability
96-
TaintTracking::localExprTaint(getPendingIntentFlagAccess("FLAG_MUTABLE"), flagArgument)
105+
TaintTracking::localExprTaint(any(MutablePendingIntentFlag flag).getAnAccess(), flagArg)
97106
)
98107
}
99108
}
100109

101-
private Expr getPendingIntentFlagAccess(string flagName) {
102-
exists(Field f |
103-
f.getDeclaringType() instanceof PendingIntent and
104-
f.isPublic() and
105-
f.isFinal() and
106-
f.hasName(flagName) and
107-
f.getAnAccess() = result
108-
)
110+
private class PendingIntentFlag extends Field {
111+
PendingIntentFlag() {
112+
this.getDeclaringType() instanceof PendingIntent and
113+
this.isPublic() and
114+
this.isFinal() and
115+
this.getName().matches("FLAG_%")
116+
}
117+
}
118+
119+
private class ImmutablePendingIntentFlag extends PendingIntentFlag {
120+
ImmutablePendingIntentFlag() { this.hasName("FLAG_IMMUTABLE") }
121+
}
122+
123+
private class MutablePendingIntentFlag extends PendingIntentFlag {
124+
MutablePendingIntentFlag() { this.hasName("FLAG_MUTABLE") }
109125
}

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

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22

33
import java.io.FileNotFoundException;
44
import android.app.Activity;
5+
import android.app.Notification;
6+
import android.app.NotificationManager;
57
import android.app.PendingIntent;
8+
import android.content.ComponentName;
69
import android.content.Context;
710
import android.content.Intent;
811
import android.content.res.AssetFileDescriptor;
@@ -19,7 +22,8 @@
1922

2023
public class ImplicitPendingIntentsTest {
2124

22-
public static void test(Context ctx) throws PendingIntent.CanceledException {
25+
public static void testPendingIntentAsAnExtra(Context ctx)
26+
throws PendingIntent.CanceledException {
2327
{
2428
Intent baseIntent = new Intent();
2529
PendingIntent pi = PendingIntent.getActivity(ctx, 0, baseIntent, 0);
@@ -34,6 +38,63 @@ public static void test(Context ctx) throws PendingIntent.CanceledException {
3438
ctx.startActivity(fwdIntent); // Safe
3539
}
3640

41+
{
42+
Intent baseIntent = new Intent();
43+
PendingIntent pi = PendingIntent.getActivityAsUser(ctx, 0, baseIntent, 0, null, null);
44+
Intent fwdIntent = new Intent();
45+
fwdIntent.putExtra("fwdIntent", pi);
46+
ctx.startActivity(fwdIntent); // $hasTaintFlow
47+
}
48+
49+
{
50+
Intent baseIntent = new Intent();
51+
PendingIntent pi = PendingIntent.getActivities(ctx, 0, new Intent[] {baseIntent}, 0);
52+
Intent fwdIntent = new Intent();
53+
fwdIntent.putExtra("fwdIntent", pi);
54+
ctx.startActivity(fwdIntent); // $hasTaintFlow
55+
}
56+
57+
{
58+
Intent baseIntent = new Intent();
59+
PendingIntent pi = PendingIntent.getActivitiesAsUser(ctx, 0, new Intent[] {baseIntent},
60+
0, null, null);
61+
Intent fwdIntent = new Intent();
62+
fwdIntent.putExtra("fwdIntent", pi);
63+
ctx.startActivity(fwdIntent); // $hasTaintFlow
64+
}
65+
66+
{
67+
Intent baseIntent = new Intent();
68+
PendingIntent pi = PendingIntent.getBroadcast(ctx, 0, baseIntent, 0);
69+
Intent fwdIntent = new Intent();
70+
fwdIntent.putExtra("fwdIntent", pi);
71+
ctx.sendBroadcast(fwdIntent); // $hasTaintFlow
72+
}
73+
74+
{
75+
Intent baseIntent = new Intent();
76+
PendingIntent pi = PendingIntent.getBroadcastAsUser(ctx, 0, baseIntent, 0, null);
77+
Intent fwdIntent = new Intent();
78+
fwdIntent.putExtra("fwdIntent", pi);
79+
ctx.sendBroadcast(fwdIntent); // $hasTaintFlow
80+
}
81+
82+
{
83+
Intent baseIntent = new Intent();
84+
PendingIntent pi = PendingIntent.getService(ctx, 0, baseIntent, 0);
85+
Intent fwdIntent = new Intent();
86+
fwdIntent.putExtra("fwdIntent", pi);
87+
ctx.startActivity(fwdIntent); // $hasTaintFlow
88+
}
89+
90+
{
91+
Intent baseIntent = new Intent();
92+
PendingIntent pi = PendingIntent.getForegroundService(ctx, 0, baseIntent, 0);
93+
Intent fwdIntent = new Intent();
94+
fwdIntent.putExtra("fwdIntent", pi);
95+
ctx.startActivity(fwdIntent); // $hasTaintFlow
96+
}
97+
3798
{
3899
Intent safeIntent = new Intent(ctx, Activity.class); // Sanitizer
39100
PendingIntent pi = PendingIntent.getActivity(ctx, 0, safeIntent, 0);
@@ -85,6 +146,51 @@ public static void test(Context ctx) throws PendingIntent.CanceledException {
85146
fwdIntent.putExtra("fwdIntent", pi);
86147
ctx.startActivity(fwdIntent); // $ SPURIOUS: $ hasTaintFlow
87148
}
149+
}
150+
151+
public static void testPendingIntentInANotification(Context ctx)
152+
throws PendingIntent.CanceledException {
153+
154+
{
155+
Intent baseIntent = new Intent();
156+
PendingIntent pi = PendingIntent.getActivity(ctx, 0, baseIntent, 0);
157+
Notification.Action.Builder aBuilder = new Notification.Action.Builder(0, "", pi);
158+
Notification.Builder nBuilder =
159+
new Notification.Builder(ctx).addAction(aBuilder.build());
160+
Notification notification = nBuilder.build();
161+
NotificationManager nManager = new NotificationManager();
162+
nManager.notifyAsPackage("targetPackage", "tag", 0, notification); // $hasTaintFlow
163+
nManager.notify(0, notification); // $hasTaintFlow
164+
nManager.notifyAsUser("", 0, notification, null); // $hasTaintFlow
165+
}
166+
{
167+
Intent baseIntent = new Intent();
168+
PendingIntent pi =
169+
PendingIntent.getActivity(ctx, 0, baseIntent, PendingIntent.FLAG_IMMUTABLE); // Sanitizer
170+
Notification.Action.Builder aBuilder = new Notification.Action.Builder(0, "", pi);
171+
Notification.Builder nBuilder =
172+
new Notification.Builder(ctx).addAction(aBuilder.build());
173+
Notification notification = nBuilder.build();
174+
NotificationManager nManager = new NotificationManager();
175+
nManager.notify(0, notification); // Safe
176+
}
177+
{
178+
// Even though pi1 is vulnerable, it's wrapped in fwdIntent,
179+
// from which pi2 (safe) is created. Since only system apps can extract an Intent
180+
// from a PendingIntent (via android.permission.GET_INTENT_SENDER_INTENT),
181+
// the attacker has no way of accessing fwdIntent, and thus pi1.
182+
Intent baseIntent = new Intent();
183+
PendingIntent pi1 = PendingIntent.getActivity(ctx, 0, baseIntent, 0);
184+
Intent fwdIntent = new Intent();
185+
fwdIntent.putExtra("fwdIntent", pi1);
186+
PendingIntent pi2 =
187+
PendingIntent.getActivity(ctx, 0, fwdIntent, PendingIntent.FLAG_IMMUTABLE);
188+
Notification.Action action = new Notification.Action(0, "", pi2);
189+
Notification.Builder nBuilder = new Notification.Builder(ctx).addAction(action);
190+
Notification notification = nBuilder.build();
191+
NotificationManager noMan = new NotificationManager();
192+
noMan.notify(0, notification); // Safe
193+
}
88194

89195
}
90196

0 commit comments

Comments
 (0)