Notification: Fix PendingIntent whitelisting
Fixes several issues with the way PendingIntents are being whitelisted
from background check with Notifications. Most visibly, this causes the
whitelisting not to work on Notifications that use the DecoratedContentViewStyle,
but there are some conditions where the whitelisting breaks for regular
notifications too.
- After a Notification is rebuilt with Notification.Builder, the set
of PendingIntents in the notification was not rebuilt.
This broke the whitelisting whenever a PendingIntent is added after
the Notification was already sent once. Workaround for broken platform
releases: always use a fresh Notification object, or clone before
sending.
- Fixes PendingIntent.writePendingIntentOrNullToParcel to invoke the
OnMarshalListener.
This broke whitelisting for any PendingIntents attached to custom
RemoteViews. Workaround for broken platform releases: Also attach
the PendingIntent to the Notification's extras.
- Changes RemoteViews to keep the parcel cookies that were present
during unparceling, such that they can be reapplied when it gets
cloned.
This broke whitelisting for any PendingIntents attached to a
DecoratedContentViewStyle *even if added to extras*. Workaround
for broken platform releases: none.
- Fixes Notification.whitelistToken mistakenly being static.
There's an unlikely race condition where the field could be
overriden with null by an incoming notification right as another
notification is sent out. Workaround for broken platform releases:
none.
Test: runtest -x core/tests/coretests/src/android/app/NotificationTest.java && runtest -x core/tests/coretests/src/android/widget/RemoteViewsTest.java
Bug: 68218899
Change-Id: I02e44040604a1d24422340611ae9e0332a611800
diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java
index fee7d6c..8226e0f 100644
--- a/core/java/android/app/Notification.java
+++ b/core/java/android/app/Notification.java
@@ -858,7 +858,7 @@
*
* @hide
*/
- static public IBinder whitelistToken;
+ private IBinder mWhitelistToken;
/**
* Must be set by a process to start associating tokens with Notification objects
@@ -1876,12 +1876,12 @@
{
int version = parcel.readInt();
- whitelistToken = parcel.readStrongBinder();
- if (whitelistToken == null) {
- whitelistToken = processWhitelistToken;
+ mWhitelistToken = parcel.readStrongBinder();
+ if (mWhitelistToken == null) {
+ mWhitelistToken = processWhitelistToken;
}
// Propagate this token to all pending intents that are unmarshalled from the parcel.
- parcel.setClassCookie(PendingIntent.class, whitelistToken);
+ parcel.setClassCookie(PendingIntent.class, mWhitelistToken);
when = parcel.readLong();
creationTime = parcel.readLong();
@@ -1989,7 +1989,7 @@
* @hide
*/
public void cloneInto(Notification that, boolean heavy) {
- that.whitelistToken = this.whitelistToken;
+ that.mWhitelistToken = this.mWhitelistToken;
that.when = this.when;
that.creationTime = this.creationTime;
that.mSmallIcon = this.mSmallIcon;
@@ -2219,7 +2219,7 @@
private void writeToParcelImpl(Parcel parcel, int flags) {
parcel.writeInt(1);
- parcel.writeStrongBinder(whitelistToken);
+ parcel.writeStrongBinder(mWhitelistToken);
parcel.writeLong(when);
parcel.writeLong(creationTime);
if (mSmallIcon == null && icon != 0) {
@@ -4981,6 +4981,8 @@
mN.flags |= FLAG_SHOW_LIGHTS;
}
+ mN.allPendingIntents = null;
+
return mN;
}
diff --git a/core/java/android/app/PendingIntent.java b/core/java/android/app/PendingIntent.java
index a25c226..4b99f3e 100644
--- a/core/java/android/app/PendingIntent.java
+++ b/core/java/android/app/PendingIntent.java
@@ -1119,8 +1119,13 @@
*/
public static void writePendingIntentOrNullToParcel(@Nullable PendingIntent sender,
@NonNull Parcel out) {
- out.writeStrongBinder(sender != null ? sender.mTarget.asBinder()
- : null);
+ out.writeStrongBinder(sender != null ? sender.mTarget.asBinder() : null);
+ if (sender != null) {
+ OnMarshaledListener listener = sOnMarshaledListener.get();
+ if (listener != null) {
+ listener.onMarshaled(sender, out, 0 /* flags */);
+ }
+ }
}
/**
diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java
index 857e8a6..c2cf3967 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -561,6 +561,22 @@
mClassCookies = from.mClassCookies;
}
+ /** @hide */
+ public Map<Class, Object> copyClassCookies() {
+ return new ArrayMap<>(mClassCookies);
+ }
+
+ /** @hide */
+ public void putClassCookies(Map<Class, Object> cookies) {
+ if (cookies == null) {
+ return;
+ }
+ if (mClassCookies == null) {
+ mClassCookies = new ArrayMap<>();
+ }
+ mClassCookies.putAll(cookies);
+ }
+
/**
* Report whether the parcel contains any marshalled file descriptors.
*/
diff --git a/core/java/android/widget/RemoteViews.java b/core/java/android/widget/RemoteViews.java
index 631f388..e3309161 100644
--- a/core/java/android/widget/RemoteViews.java
+++ b/core/java/android/widget/RemoteViews.java
@@ -81,6 +81,7 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.Map;
import java.util.Stack;
import java.util.concurrent.Executor;
@@ -185,6 +186,9 @@
*/
private boolean mIsWidgetCollectionChild = false;
+ /** Class cookies of the Parcel this instance was read from. */
+ private final Map<Class, Object> mClassCookies;
+
private static final OnClickHandler DEFAULT_ON_CLICK_HANDLER = new OnClickHandler();
private static final ArrayMap<MethodKey, MethodArgs> sMethods = new ArrayMap<>();
@@ -1505,10 +1509,10 @@
}
ViewGroupActionAdd(Parcel parcel, BitmapCache bitmapCache, ApplicationInfo info,
- int depth) {
+ int depth, Map<Class, Object> classCookies) {
viewId = parcel.readInt();
mIndex = parcel.readInt();
- mNestedViews = new RemoteViews(parcel, bitmapCache, info, depth);
+ mNestedViews = new RemoteViews(parcel, bitmapCache, info, depth, classCookies);
}
public void writeToParcel(Parcel dest, int flags) {
@@ -2120,6 +2124,7 @@
mApplication = application;
mLayoutId = layoutId;
mBitmapCache = new BitmapCache();
+ mClassCookies = null;
}
private boolean hasLandscapeAndPortraitLayouts() {
@@ -2149,6 +2154,9 @@
mBitmapCache = new BitmapCache();
configureRemoteViewsAsChild(landscape);
configureRemoteViewsAsChild(portrait);
+
+ mClassCookies = (portrait.mClassCookies != null)
+ ? portrait.mClassCookies : landscape.mClassCookies;
}
/**
@@ -2161,15 +2169,16 @@
mLayoutId = src.mLayoutId;
mIsWidgetCollectionChild = src.mIsWidgetCollectionChild;
mReapplyDisallowed = src.mReapplyDisallowed;
+ mClassCookies = src.mClassCookies;
if (src.hasLandscapeAndPortraitLayouts()) {
mLandscape = new RemoteViews(src.mLandscape);
mPortrait = new RemoteViews(src.mPortrait);
-
}
if (src.mActions != null) {
Parcel p = Parcel.obtain();
+ p.putClassCookies(mClassCookies);
src.writeActionsToParcel(p);
p.setDataPosition(0);
// Since src is already in memory, we do not care about stack overflow as it has
@@ -2189,10 +2198,11 @@
* @param parcel
*/
public RemoteViews(Parcel parcel) {
- this(parcel, null, null, 0);
+ this(parcel, null, null, 0, null);
}
- private RemoteViews(Parcel parcel, BitmapCache bitmapCache, ApplicationInfo info, int depth) {
+ private RemoteViews(Parcel parcel, BitmapCache bitmapCache, ApplicationInfo info, int depth,
+ Map<Class, Object> classCookies) {
if (depth > MAX_NESTED_VIEWS
&& (UserHandle.getAppId(Binder.getCallingUid()) != Process.SYSTEM_UID)) {
throw new IllegalArgumentException("Too many nested views.");
@@ -2204,8 +2214,11 @@
// We only store a bitmap cache in the root of the RemoteViews.
if (bitmapCache == null) {
mBitmapCache = new BitmapCache(parcel);
+ // Store the class cookies such that they are available when we clone this RemoteView.
+ mClassCookies = parcel.copyClassCookies();
} else {
setBitmapCache(bitmapCache);
+ mClassCookies = classCookies;
setNotRoot();
}
@@ -2218,8 +2231,9 @@
readActionsFromParcel(parcel, depth);
} else {
// MODE_HAS_LANDSCAPE_AND_PORTRAIT
- mLandscape = new RemoteViews(parcel, mBitmapCache, info, depth);
- mPortrait = new RemoteViews(parcel, mBitmapCache, mLandscape.mApplication, depth);
+ mLandscape = new RemoteViews(parcel, mBitmapCache, info, depth, mClassCookies);
+ mPortrait = new RemoteViews(parcel, mBitmapCache, mLandscape.mApplication, depth,
+ mClassCookies);
mApplication = mPortrait.mApplication;
mLayoutId = mPortrait.getLayoutId();
}
@@ -2246,7 +2260,8 @@
case REFLECTION_ACTION_TAG:
return new ReflectionAction(parcel);
case VIEW_GROUP_ACTION_ADD_TAG:
- return new ViewGroupActionAdd(parcel, mBitmapCache, mApplication, depth);
+ return new ViewGroupActionAdd(parcel, mBitmapCache, mApplication, depth,
+ mClassCookies);
case VIEW_GROUP_ACTION_REMOVE_TAG:
return new ViewGroupActionRemove(parcel);
case VIEW_CONTENT_NAVIGATION_TAG:
diff --git a/core/tests/coretests/src/android/app/NotificationTest.java b/core/tests/coretests/src/android/app/NotificationTest.java
index 5457713..bec862a 100644
--- a/core/tests/coretests/src/android/app/NotificationTest.java
+++ b/core/tests/coretests/src/android/app/NotificationTest.java
@@ -22,10 +22,13 @@
import static org.junit.Assert.assertTrue;
import android.content.Context;
+import android.content.Intent;
import android.media.session.MediaSession;
+import android.os.Parcel;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
import android.support.test.runner.AndroidJUnit4;
+import android.widget.RemoteViews;
import org.junit.Before;
import org.junit.Test;
@@ -138,6 +141,44 @@
assertFalse(n.hasCompletedProgress());
}
+ @Test
+ public void allPendingIntents_recollectedAfterReusingBuilder() {
+ PendingIntent intent1 = PendingIntent.getActivity(mContext, 0, new Intent("test1"), 0);
+ PendingIntent intent2 = PendingIntent.getActivity(mContext, 0, new Intent("test2"), 0);
+
+ Notification.Builder builder = new Notification.Builder(mContext, "channel");
+ builder.setContentIntent(intent1);
+
+ Parcel p = Parcel.obtain();
+
+ Notification n1 = builder.build();
+ n1.writeToParcel(p, 0);
+
+ builder.setContentIntent(intent2);
+ Notification n2 = builder.build();
+ n2.writeToParcel(p, 0);
+
+ assertTrue(n2.allPendingIntents.contains(intent2));
+ }
+
+ @Test
+ public void allPendingIntents_containsCustomRemoteViews() {
+ PendingIntent intent = PendingIntent.getActivity(mContext, 0, new Intent("test"), 0);
+
+ RemoteViews contentView = new RemoteViews(mContext.getPackageName(), 0 /* layoutId */);
+ contentView.setOnClickPendingIntent(1 /* id */, intent);
+
+ Notification.Builder builder = new Notification.Builder(mContext, "channel");
+ builder.setCustomContentView(contentView);
+
+ Parcel p = Parcel.obtain();
+
+ Notification n = builder.build();
+ n.writeToParcel(p, 0);
+
+ assertTrue(n.allPendingIntents.contains(intent));
+ }
+
private Notification.Builder getMediaNotification() {
MediaSession session = new MediaSession(mContext, "test");
return new Notification.Builder(mContext, "color")
diff --git a/core/tests/coretests/src/android/widget/RemoteViewsTest.java b/core/tests/coretests/src/android/widget/RemoteViewsTest.java
index ddf9876..70cf097 100644
--- a/core/tests/coretests/src/android/widget/RemoteViewsTest.java
+++ b/core/tests/coretests/src/android/widget/RemoteViewsTest.java
@@ -27,6 +27,7 @@
import android.graphics.drawable.BitmapDrawable;
import android.graphics.drawable.Drawable;
import android.os.AsyncTask;
+import android.os.Binder;
import android.os.Parcel;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
@@ -376,13 +377,24 @@
parcelAndRecreate(views);
}
- private void parcelAndRecreate(RemoteViews views) {
- Parcel p = Parcel.obtain();
- views.writeToParcel(p, 0);
- p.setDataPosition(0);
+ private RemoteViews parcelAndRecreate(RemoteViews views) {
+ return parcelAndRecreateWithPendingIntentCookie(views, null);
+ }
- RemoteViews.CREATOR.createFromParcel(p);
- p.recycle();
+ private RemoteViews parcelAndRecreateWithPendingIntentCookie(RemoteViews views, Object cookie) {
+ Parcel p = Parcel.obtain();
+ try {
+ views.writeToParcel(p, 0);
+ p.setDataPosition(0);
+
+ if (cookie != null) {
+ p.setClassCookie(PendingIntent.class, cookie);
+ }
+
+ return RemoteViews.CREATOR.createFromParcel(p);
+ } finally {
+ p.recycle();
+ }
}
@Test
@@ -399,4 +411,37 @@
throw new Exception(t);
}
}
+
+ @Test
+ public void copy_keepsPendingIntentWhitelistToken() throws Exception {
+ Binder whitelistToken = new Binder();
+
+ RemoteViews views = new RemoteViews(mPackage, R.layout.remote_views_test);
+ PendingIntent pi = PendingIntent.getBroadcast(mContext, 0,
+ new Intent("test"), PendingIntent.FLAG_ONE_SHOT);
+ views.setOnClickPendingIntent(1, pi);
+ RemoteViews withCookie = parcelAndRecreateWithPendingIntentCookie(views, whitelistToken);
+
+ RemoteViews cloned = new RemoteViews(withCookie);
+
+ PendingIntent found = extractAnyPendingIntent(cloned);
+ assertEquals(whitelistToken, found.getWhitelistToken());
+ }
+
+ private PendingIntent extractAnyPendingIntent(RemoteViews cloned) {
+ PendingIntent[] found = new PendingIntent[1];
+ Parcel p = Parcel.obtain();
+ try {
+ PendingIntent.setOnMarshaledListener((intent, parcel, flags) -> {
+ if (parcel == p) {
+ found[0] = intent;
+ }
+ });
+ cloned.writeToParcel(p, 0);
+ } finally {
+ p.recycle();
+ PendingIntent.setOnMarshaledListener(null);
+ }
+ return found[0];
+ }
}