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

Commit a7cae90a authored by Winson Chung's avatar Winson Chung
Browse files

Restricts callers to fetch activity icons for their own apps

- With the exception of SysUI, callers should only be able
  to fetch icons set on their own task descriptions
- Removing unused code to fetch task description for a given task

Bug: 318087006
Test: atest ActivityTaskManagerServiceTests
Test: Verify with POC apks in the bug
Change-Id: Ie7822e5701064fc3bc2e3cfb94c541ea21d02228
parent 04b951c1
Loading
Loading
Loading
Loading
+0 −1
Original line number Diff line number Diff line
@@ -157,7 +157,6 @@ interface IActivityTaskManager {
    ParceledListSlice<ActivityManager.RecentTaskInfo> getRecentTasks(int maxNum, int flags,
            int userId);
    boolean isTopActivityImmersive();
    ActivityManager.TaskDescription getTaskDescription(int taskId);
    void reportAssistContextExtras(in IBinder assistToken, in Bundle extras,
            in AssistStructure structure, in AssistContent content, in Uri referrer);

+34 −15
Original line number Diff line number Diff line
@@ -2160,19 +2160,6 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {
        return rect;
    }

    @Override
    public ActivityManager.TaskDescription getTaskDescription(int id) {
        synchronized (mGlobalLock) {
            enforceTaskPermission("getTaskDescription()");
            final Task tr = mRootWindowContainer.anyTaskForId(id,
                    MATCH_ATTACHED_TASK_OR_RECENT_TASKS);
            if (tr != null) {
                return tr.getTaskDescription();
            }
        }
        return null;
    }

    /**
     * Sets the locusId for a particular activity.
     *
@@ -3072,8 +3059,33 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {

    @Override
    public Bitmap getTaskDescriptionIcon(String filePath, int userId) {
        userId = handleIncomingUser(Binder.getCallingPid(), Binder.getCallingUid(),
                userId, "getTaskDescriptionIcon");
        final int callingUid = Binder.getCallingUid();
        // Verify that the caller can make the request for the given userId
        userId = handleIncomingUser(Binder.getCallingPid(), callingUid, userId,
                "getTaskDescriptionIcon");
        synchronized (mGlobalLock) {
            // Verify that the caller can make the request for given icon file path
            final ActivityRecord matchingActivity = mRootWindowContainer.getActivity(
                    r -> {
                        if (r.taskDescription == null
                                || r.taskDescription.getIconFilename() == null) {
                            return false;
                        }
                        return r.taskDescription.getIconFilename().equals(filePath);
                    });
            if (matchingActivity == null || (matchingActivity.getUid() != callingUid)) {
                // Caller UID doesn't match the requested Activity's UID, check if caller is
                // privileged
                try {
                    enforceActivityTaskPermission("getTaskDescriptionIcon");
                } catch (SecurityException e) {
                    Slog.w(TAG, "getTaskDescriptionIcon(): request (callingUid=" + callingUid
                            + ", filePath=" + filePath + ", user=" + userId + ") doesn't match any "
                            + "activity");
                    throw e;
                }
            }
        }

        final File passedIconFile = new File(filePath);
        final File legitIconFile = new File(TaskPersister.getUserImagesDir(userId),
@@ -3303,6 +3315,13 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {
        return false;
    }

    /**
     * An instance method that's easier for mocking in tests.
     */
    void enforceActivityTaskPermission(String func) {
        enforceTaskPermission(func);
    }

    static void enforceTaskPermission(String func) {
        if (checkCallingPermission(MANAGE_ACTIVITY_TASKS) == PackageManager.PERMISSION_GRANTED) {
            return;
+62 −0
Original line number Diff line number Diff line
@@ -38,18 +38,22 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.Activity;
import android.app.ActivityManager;
import android.app.ActivityManager.TaskDescription;
import android.app.IApplicationThread;
import android.app.PictureInPictureParams;
import android.app.servertransaction.ClientTransactionItem;
@@ -62,6 +66,7 @@ import android.os.Binder;
import android.os.LocaleList;
import android.os.PowerManagerInternal;
import android.os.RemoteException;
import android.os.UserHandle;
import android.platform.test.annotations.Presubmit;
import android.view.Display;
import android.view.DisplayInfo;
@@ -1099,4 +1104,61 @@ public class ActivityTaskManagerServiceTests extends WindowTestsBase {

        verify(mClientLifecycleManager).onLayoutContinued();
    }

    @Test
    public void testGetTaskDescriptionIcon_matchingUid() {
        // Ensure that we do not hold MANAGE_ACTIVITY_TASKS
        doThrow(new SecurityException()).when(mAtm).enforceActivityTaskPermission(any());

        final String filePath = "abc/def";
        // Create an activity with a task description at the test icon filepath
        final ActivityRecord activity = new ActivityBuilder(mAtm)
                .setUid(android.os.Process.myUid())
                .setCreateTask(true)
                .build();
        final TaskDescription td = new TaskDescription.Builder().build();
        td.setIconFilename(filePath);
        activity.setTaskDescription(td);

        // Verify this calls and does not throw a security exception
        try {
            mAtm.getTaskDescriptionIcon(filePath, activity.mUserId);
        } catch (SecurityException e) {
            fail("Unexpected security exception: " + e);
        } catch (IllegalArgumentException e) {
            // Ok, the file doesn't actually exist
        }
    }

    @Test
    public void testGetTaskDescriptionIcon_noMatchingActivity_expectException() {
        // Ensure that we do not hold MANAGE_ACTIVITY_TASKS
        doThrow(new SecurityException()).when(mAtm).enforceActivityTaskPermission(any());

        final String filePath = "abc/def";

        // Verify this throws a security exception due to no matching activity
        assertThrows(SecurityException.class,
                () -> mAtm.getTaskDescriptionIcon(filePath, UserHandle.myUserId()));
    }

    @Test
    public void testGetTaskDescriptionIcon_noMatchingUid_expectException() {
        // Ensure that we do not hold MANAGE_ACTIVITY_TASKS
        doThrow(new SecurityException()).when(mAtm).enforceActivityTaskPermission(any());

        final String filePath = "abc/def";
        // Create an activity with a task description at the test icon filepath
        final ActivityRecord activity = new ActivityBuilder(mAtm)
                .setCreateTask(true)
                .setUid(101010)
                .build();
        final TaskDescription td = new TaskDescription.Builder().build();
        td.setIconFilename(filePath);
        activity.setTaskDescription(td);

        // Verify this throws a security exception due to no matching UID
        assertThrows(SecurityException.class,
                () -> mAtm.getTaskDescriptionIcon(filePath, activity.mUserId));
    }
}
+0 −1
Original line number Diff line number Diff line
@@ -1472,7 +1472,6 @@ public class RecentTasksTest extends WindowTestsBase {
        assertSecurityException(expectCallable, () -> mAtm.registerTaskStackListener(null));
        assertSecurityException(expectCallable,
                () -> mAtm.unregisterTaskStackListener(null));
        assertSecurityException(expectCallable, () -> mAtm.getTaskDescription(0));
        assertSecurityException(expectCallable, () -> mAtm.cancelTaskWindowTransition(0));
        assertSecurityException(expectCallable, () -> mAtm.startRecentsActivity(null, 0,
                null));