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

Commit d029091b authored by oli's avatar oli Committed by mse1969
Browse files

Add resolveActivityAsUserForExplicitType api to pm 

allow checking for resolved activity for an intent with a specific type

allowing the intent to declare its own type can lead to a security
vulnerability if the intent changes its type after the
IntentForwarder#canForward check.

Use the new API in IntentForwarderActivity to prevent checking the
intent's type twice.

Update other call sites of IntentForwarder#canForward to extract the
resolved type from the intent and pass in to to the method call

Check the launch package rather than calling package

sanitize intent selector as well as received intent

Bug: 403565650 407763772 397216638
Flag: EXEMPT bugfix
Test: manually tested
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:57233409d9bb5aef50373760a9b6551aa618c808)
Cherrypick-From: https://googleplex-android-review.googlesource.com/q/commit:e80fafa79999bdcd623e4393d528cf0ac8db02fd
Merged-In: I8453f25ed5ddd938718f4d2be4983c881778f281
Change-Id: I8453f25ed5ddd938718f4d2be4983c881778f281
parent 42eecf88

core/java/android/app/ApplicationPackageManager.java

+21 −7
Original line number Diff line number Diff line
@@ -1398,22 +1398,36 @@ public class ApplicationPackageManager extends PackageManager { 


    @Override

    public ResolveInfo resolveActivity(Intent intent, ResolveInfoFlags flags) {

        return resolveActivityAsUser(intent, flags, getUserId());

        return resolveActivityAsUser(intent, /* resolvedType= */ null, flags, getUserId());

    }



    @Override

    public ResolveInfo resolveActivityAsUser(Intent intent, int flags, int userId) {

        return resolveActivityAsUser(intent, ResolveInfoFlags.of(flags), userId);

        return resolveActivityAsUser(intent, /* resolvedType= */ null, ResolveInfoFlags.of(flags),

                userId);

    }



    @Override

    public ResolveInfo resolveActivityAsUser(Intent intent, ResolveInfoFlags flags, int userId) {

        return resolveActivityAsUser(intent, /* resolvedType= */ null, flags, userId);

    }



    @Override

    public ResolveInfo resolveActivityAsUser(Intent intent, String resolvedType,

            int flags, int userId) {

        return resolveActivityAsUser(intent, resolvedType,

                ResolveInfoFlags.of(flags), userId);

    }



    @Override

    public ResolveInfo resolveActivityAsUser(Intent intent, String resolvedType,

            ResolveInfoFlags flags, int userId) {

        try {

            return mPM.resolveIntent(

                intent,

                intent.resolveTypeIfNeeded(mContext.getContentResolver()),

                updateFlagsForComponent(flags.getValue(), userId, intent),

                userId);

            return mPM.resolveIntent(intent,

                    resolvedType == null

                        ? intent.resolveTypeIfNeeded(mContext.getContentResolver())

                        : resolvedType,

                    updateFlagsForComponent(flags.getValue(), userId, intent), userId);

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

        }

core/java/android/content/pm/PackageManager.java

+29 −1
Original line number Diff line number Diff line
@@ -6868,8 +6868,16 @@ public abstract class PackageManager { 
     * Intent.resolveActivity(PackageManager)} do.

     * </p>

     *

     * Use {@link #resolveActivityAsUser(Intent, String, ResolveInfoFlags, int)}

     * when long flags are needed.

     *

     * @param intent An intent containing all of the desired specification

     *            (action, data, type, category, and/or component).

     * @param resolvedType A nullable resolved type for the intent's data.

     *            Specified explicitly when the data type contained in the

     *            intent cannot be trusted. If null is provided, the type will

     *            be obtained from the intent using

     *            {@link Intent#resolveTypeIfNeeded}.

     * @param flags Additional option flags to modify the data returned. The

     *            most important is {@link #MATCH_DEFAULT_ONLY}, to limit the

     *            resolution to only those activities that support the
@@ -6884,6 +6892,26 @@ public abstract class PackageManager { 
     * @deprecated Use {@link #resolveActivityAsUser(Intent, ResolveInfoFlags, int)} instead.

     */

    @Deprecated

    @Nullable

    public ResolveInfo resolveActivityAsUser(@NonNull Intent intent,

            @Nullable String resolvedType, int flags, @UserIdInt int userId) {

        return resolveActivityAsUser(intent, resolvedType, ResolveInfoFlags.of(flags), userId);

    }



    /**

     * See {@link #resolveActivityAsUser(Intent, String, int, int)}.

     * @hide

     */

    @Nullable

    public ResolveInfo resolveActivityAsUser(@NonNull Intent intent,

            @Nullable String resolvedType, @NonNull ResolveInfoFlags flags, @UserIdInt int userId) {

        throw new UnsupportedOperationException(

                "resolveActivityAsUser not implemented in subclass");

    }

     /**

     * See {@link #resolveActivityAsUser(Intent, String, int, int)}.

     * @hide

     */

    @SuppressWarnings("HiddenAbstractMethod")

    @Nullable

    @UnsupportedAppUsage
@@ -6891,7 +6919,7 @@ public abstract class PackageManager { 
            int flags, @UserIdInt int userId);



    /**

     * See {@link #resolveActivityAsUser(Intent, int, int)}.

     * See {@link #resolveActivityAsUser(Intent, String, int, int)}.

     * @hide

     */

    @Nullable

core/java/com/android/internal/app/AbstractMultiProfilePagerAdapter.java

+1 −1
Original line number Diff line number Diff line
@@ -351,7 +351,7 @@ public abstract class AbstractMultiProfilePagerAdapter extends PagerAdapter { 


            return intents.stream().anyMatch(intent ->

                    null != IntentForwarderActivity.canForward(intent, source, target,

                            packageManager, mContentResolver));

                            packageManager, intent.resolveTypeIfNeeded(mContentResolver)));

        }

    }

core/java/com/android/internal/app/IntentForwarderActivity.java

+26 −8
Original line number Diff line number Diff line
@@ -30,7 +30,6 @@ import android.app.AppGlobals; 
import android.app.admin.DevicePolicyManager;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.ComponentName;

import android.content.ContentResolver;

import android.content.Intent;

import android.content.pm.ActivityInfo;

import android.content.pm.IPackageManager;
@@ -133,8 +132,9 @@ public class IntentForwarderActivity extends Activity { 
        }



        final int callingUserId = getUserId();

        String resolvedType = intentReceived.resolveTypeIfNeeded(getContentResolver());

        final Intent newIntent = canForward(intentReceived, getUserId(), targetUserId,

                mInjector.getIPackageManager(), getContentResolver());

                mInjector.getIPackageManager(), resolvedType);



        if (newIntent == null) {

            Slog.wtf(TAG, "the intent: " + intentReceived + " cannot be forwarded from user "
@@ -145,7 +145,11 @@ public class IntentForwarderActivity extends Activity { 


        newIntent.prepareToLeaveUser(callingUserId);

        final CompletableFuture<ResolveInfo> targetResolveInfoFuture =

                mInjector.resolveActivityAsUser(newIntent, MATCH_DEFAULT_ONLY, targetUserId);

                mInjector.resolveActivityAsUser(

                        newIntent,

                        resolvedType,

                        MATCH_DEFAULT_ONLY,

                        targetUserId);

        targetResolveInfoFuture

                .thenApplyAsync(targetResolveInfo -> {

                    if (isResolverActivityResolveInfo(targetResolveInfo)) {
@@ -248,6 +252,9 @@ public class IntentForwarderActivity extends Activity { 
                ? targetUserId : callingUserId;

        int selectedProfile = findSelectedProfile(className);

        sanitizeIntent(intentReceived);

        if (intentReceived.getSelector() != null) {

            sanitizeIntent(intentReceived.getSelector());

        }

        intentReceived.putExtra(EXTRA_SELECTED_PROFILE, selectedProfile);

        intentReceived.putExtra(EXTRA_CALLING_USER, UserHandle.of(callingUserId));

        startActivityAsCaller(intentReceived, null, false, userId);
@@ -313,20 +320,20 @@ public class IntentForwarderActivity extends Activity { 
     * forwarding if it can be forwarded, {@code null} otherwise.

     */

    static Intent canForward(Intent incomingIntent, int sourceUserId, int targetUserId,

            IPackageManager packageManager, ContentResolver contentResolver)  {

            IPackageManager packageManager, String resolvedType)  {

        Intent forwardIntent = new Intent(incomingIntent);

        forwardIntent.addFlags(

                Intent.FLAG_ACTIVITY_FORWARD_RESULT | Intent.FLAG_ACTIVITY_PREVIOUS_IS_TOP);

        sanitizeIntent(forwardIntent);



        if (!canForwardInner(forwardIntent, sourceUserId, targetUserId, packageManager,

                contentResolver)) {

                resolvedType)) {

            return null;

        }

        if (forwardIntent.getSelector() != null) {

            sanitizeIntent(forwardIntent.getSelector());

            if (!canForwardInner(forwardIntent.getSelector(), sourceUserId, targetUserId,

                    packageManager, contentResolver)) {

                    packageManager, resolvedType)) {

                return null;

            }

        }
@@ -334,11 +341,10 @@ public class IntentForwarderActivity extends Activity { 
    }



    private static boolean canForwardInner(Intent intent, int sourceUserId, int targetUserId,

            IPackageManager packageManager, ContentResolver contentResolver) {

            IPackageManager packageManager, String resolvedType) {

        if (Intent.ACTION_CHOOSER.equals(intent.getAction())) {

            return false;

        }

        String resolvedType = intent.resolveTypeIfNeeded(contentResolver);

        try {

            if (packageManager.canForwardTo(

                    intent, resolvedType, sourceUserId, targetUserId)) {
@@ -419,6 +425,15 @@ public class IntentForwarderActivity extends Activity { 
            return IntentForwarderActivity.this.getPackageManager();

        }



        @Override

        @Nullable

        public CompletableFuture<ResolveInfo> resolveActivityAsUser(Intent intent,

                String resolvedType, int flags, int userId) {

            return CompletableFuture.supplyAsync(

                    () -> getPackageManager().resolveActivityAsUser(intent,

                            resolvedType, flags, userId));

        }



        @Override

        @Nullable

        public CompletableFuture<ResolveInfo> resolveActivityAsUser(
@@ -440,6 +455,9 @@ public class IntentForwarderActivity extends Activity { 


        PackageManager getPackageManager();



        CompletableFuture<ResolveInfo> resolveActivityAsUser(Intent intent,

                String resolvedType, int flags, int userId);



        CompletableFuture<ResolveInfo> resolveActivityAsUser(Intent intent, int flags, int userId);



        void showToast(String message, int duration);

core/tests/coretests/src/com/android/internal/app/IntentForwarderActivityTest.java

+19 −2
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import static org.mockito.ArgumentMatchers.nullable; 
import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;
@@ -266,10 +267,20 @@ public class IntentForwarderActivityTest { 
                Intent.ACTION_VIEW, Intent.CATEGORY_BROWSABLE);

        IntentForwarderWrapperActivity activity = mActivityRule.launchActivity(intent);



        InstrumentationRegistry.getInstrumentation().waitForIdleSync();

        ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);

        verify(mIPm).canForwardTo(

        verify(mIPm, times(2)).canForwardTo(

                intentCaptor.capture(), nullable(String.class), anyInt(), anyInt());

        assertEquals(Intent.ACTION_VIEW, intentCaptor.getValue().getAction());

        List<Intent> capturedIntents = intentCaptor.getAllValues();

        // Verify root intent is checked and sanitized

        assertEquals(Intent.ACTION_MAIN, capturedIntents.get(0).getAction());

        assertNull(capturedIntents.get(0));

        assertNull(capturedIntents.get(0).getPackage());

        // Verify selector is checked and sanitized

        assertEquals(Intent.ACTION_VIEW, capturedIntents.get(1).getAction());

        assertNull(capturedIntents.get(1));

        assertNull(capturedIntents.get(1).getPackage());

        InstrumentationRegistry.getInstrumentation().waitForIdleSync();



        assertNotNull(activity.mStartActivityIntent);

        assertEquals(Intent.ACTION_MAIN, activity.mStartActivityIntent.getAction());
@@ -676,6 +687,12 @@ public class IntentForwarderActivityTest { 
            return mPm;

        }



        @Override

        public CompletableFuture<ResolveInfo> resolveActivityAsUser(Intent intent,

                String resolvedType, int flags, int userId) {

            return resolveActivityAsUser(intent, flags, userId);

        }



        @Override

        public CompletableFuture<ResolveInfo> resolveActivityAsUser(

                Intent intent, int flags, int userId) {