Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fb92184e authored by Adrian Roos's avatar Adrian Roos
Browse files

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
parent 9b874662

core/java/android/app/Notification.java

+9 −7
Original line number Diff line number Diff line
@@ -858,7 +858,7 @@ public class Notification implements Parcelable 
     *

     * @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 @@ public class Notification implements Parcelable 
    {

        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 @@ public class Notification implements Parcelable 
     * @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 @@ public class Notification implements Parcelable 
    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 @@ public class Notification implements Parcelable 
                mN.flags |= FLAG_SHOW_LIGHTS;

            }



            mN.allPendingIntents = null;



            return mN;

        }

core/java/android/app/PendingIntent.java

+7 −2
Original line number Diff line number Diff line
@@ -1119,8 +1119,13 @@ public final class PendingIntent implements Parcelable { 
     */

    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 */);

            }

        }

    }



    /**

core/java/android/os/Parcel.java

+16 −0
Original line number Diff line number Diff line
@@ -561,6 +561,22 @@ public final class Parcel { 
        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.

     */

core/java/android/widget/RemoteViews.java

+23 −8
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ import java.lang.invoke.MethodType; 
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 @@ public class RemoteViews implements Parcelable, Filter { 
     */

    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 @@ public class RemoteViews implements Parcelable, Filter { 
        }



        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 @@ public class RemoteViews implements Parcelable, Filter { 
        mApplication = application;

        mLayoutId = layoutId;

        mBitmapCache = new BitmapCache();

        mClassCookies = null;

    }



    private boolean hasLandscapeAndPortraitLayouts() {
@@ -2149,6 +2154,9 @@ public class RemoteViews implements Parcelable, Filter { 
        mBitmapCache = new BitmapCache();

        configureRemoteViewsAsChild(landscape);

        configureRemoteViewsAsChild(portrait);



        mClassCookies = (portrait.mClassCookies != null)

                ? portrait.mClassCookies : landscape.mClassCookies;

    }



    /**
@@ -2161,15 +2169,16 @@ public class RemoteViews implements Parcelable, Filter { 
        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 @@ public class RemoteViews implements Parcelable, Filter { 
     * @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 @@ public class RemoteViews implements Parcelable, Filter { 
        // 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 @@ public class RemoteViews implements Parcelable, Filter { 
            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 @@ public class RemoteViews implements Parcelable, Filter { 
            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:

core/tests/coretests/src/android/app/NotificationTest.java

+41 −0
Original line number Diff line number Diff line
@@ -22,10 +22,13 @@ import static org.junit.Assert.assertFalse; 
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 @@ public class NotificationTest { 
        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")