Merge "Fix security bug for TaskFragment creation" into sc-v2-dev (5b9b66e8) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/wm/ActivityStartController.java

+5 −3

Original line number	Diff line number	Diff line
		@@ -496,11 +496,13 @@ public class ActivityStartController {
		* @param activityIntent intent to start the activity.
		* @param activityOptions ActivityOptions to start the activity with.
		* @param resultTo the caller activity
		* @param callingUid the caller uid
		* @param callingPid the caller pid
		* @return the start result.
		*/
		int startActivityInTaskFragment(@NonNull TaskFragment taskFragment,
		@NonNull Intent activityIntent, @Nullable Bundle activityOptions,
		@Nullable IBinder resultTo) {
		@Nullable IBinder resultTo, int callingUid, int callingPid) {
		final ActivityRecord caller =
		resultTo != null ? ActivityRecord.forTokenLocked(resultTo) : null;
		return obtainStarter(activityIntent, "startActivityInTaskFragment")
		@@ -508,8 +510,8 @@ public class ActivityStartController {
		.setInTaskFragment(taskFragment)
		.setResultTo(resultTo)
		.setRequestCode(-1)
		.setCallingUid(Binder.getCallingUid())
		.setCallingPid(Binder.getCallingPid())
		.setCallingUid(callingUid)
		.setCallingPid(callingPid)
		.setUserId(caller != null ? caller.mUserId : mService.getCurrentUserId())
		.execute();
		}

services/core/java/com/android/server/wm/AppTransitionController.java

+1 −1

Original line number	Diff line number	Diff line
		@@ -590,7 +590,7 @@ public class AppTransitionController {
		}
		// We don't want the organizer to handle transition of non-embedded activity of other
		// app.
		if (r.getUid() != rootActivity.getUid() && !r.isEmbedded()) {
		if (r.getUid() != task.effectiveUid && !r.isEmbedded()) {
		leafTask = null;
		break;
		}

services/core/java/com/android/server/wm/WindowOrganizerController.java

+6 −6

Original line number	Diff line number	Diff line
		@@ -588,7 +588,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
		case HIERARCHY_OP_TYPE_CREATE_TASK_FRAGMENT: {
		final TaskFragmentCreationParams taskFragmentCreationOptions =
		hop.getTaskFragmentCreationOptions();
		createTaskFragment(taskFragmentCreationOptions, errorCallbackToken);
		createTaskFragment(taskFragmentCreationOptions, errorCallbackToken, caller);
		break;
		}
		case HIERARCHY_OP_TYPE_DELETE_TASK_FRAGMENT: {
		@@ -631,7 +631,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
		final TaskFragment tf = mLaunchTaskFragments.get(fragmentToken);
		final int result = mService.getActivityStartController()
		.startActivityInTaskFragment(tf, activityIntent, activityOptions,
		hop.getCallingActivity());
		hop.getCallingActivity(), caller.mUid, caller.mPid);
		if (!isStartResultSuccessful(result)) {
		sendTaskFragmentOperationFailure(tf.getTaskFragmentOrganizer(),
		errorCallbackToken,
		@@ -1200,7 +1200,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
		}

		void createTaskFragment(@NonNull TaskFragmentCreationParams creationParams,
		@Nullable IBinder errorCallbackToken) {
		@Nullable IBinder errorCallbackToken, @NonNull CallerInfo caller) {
		final ActivityRecord ownerActivity =
		ActivityRecord.forTokenLocked(creationParams.getOwnerToken());
		final ITaskFragmentOrganizer organizer = ITaskFragmentOrganizer.Stub.asInterface(
		@@ -1218,9 +1218,9 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
		sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
		return;
		}
		// The ownerActivity has to belong to the same app as the root Activity of the target Task.
		final ActivityRecord rootActivity = ownerActivity.getTask().getRootActivity();
		if (rootActivity.getUid() != ownerActivity.getUid()) {
		// The ownerActivity has to belong to the same app as the target Task.
		if (ownerActivity.getTask().effectiveUid != ownerActivity.getUid()
		\|\| ownerActivity.getTask().effectiveUid != caller.mUid) {
		final Throwable exception =
		new IllegalArgumentException("Not allowed to operate with the ownerToken while "
		+ "the root activity of the target task belong to the different app");

services/tests/wmtests/src/com/android/server/wm/AppTransitionControllerTest.java

+2 −0

Original line number	Diff line number	Diff line
		@@ -803,6 +803,7 @@ public class AppTransitionControllerTest extends WindowTestsBase {
		final TaskFragment taskFragment = createTaskFragmentWithEmbeddedActivity(task, organizer);
		final ActivityRecord openingActivity = taskFragment.getTopMostActivity();
		openingActivity.allDrawn = true;
		task.effectiveUid = openingActivity.getUid();
		spyOn(mDisplayContent.mAppTransition);

		// Prepare a transition.
		@@ -879,6 +880,7 @@ public class AppTransitionControllerTest extends WindowTestsBase {
		final ActivityRecord closingActivity = taskFragment.getTopMostActivity();
		closingActivity.allDrawn = true;
		closingActivity.info.applicationInfo.uid = 12345;
		task.effectiveUid = closingActivity.getUid();
		// Opening non-embedded activity with different UID.
		final ActivityRecord openingActivity = createActivityRecord(task);
		openingActivity.info.applicationInfo.uid = 54321;

services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java

+52 −13

Original line number	Diff line number	Diff line
		@@ -21,6 +21,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
		import static com.android.server.wm.testing.Assert.assertThrows;

		import static org.junit.Assert.assertEquals;
		import static org.junit.Assert.assertNotNull;
		import static org.junit.Assert.assertNull;
		import static org.junit.Assert.fail;
		import static org.mockito.ArgumentMatchers.any;
		@@ -352,28 +353,66 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
		}

		@Test
		public void testApplyTransaction_enforceHierarchyChange_createTaskFragment() {
		public void testApplyTransaction_enforceHierarchyChange_createTaskFragment()
		throws RemoteException {
		mController.registerOrganizer(mIOrganizer);
		final ActivityRecord activity = createActivityRecord(mDisplayContent);
		final int uid = Binder.getCallingUid();
		activity.info.applicationInfo.uid = uid;
		activity.getTask().effectiveUid = uid;
		final IBinder fragmentToken = new Binder();
		final TaskFragmentCreationParams params = new TaskFragmentCreationParams.Builder(
		mOrganizerToken, fragmentToken, activity.token).build();
		mOrganizer.applyTransaction(mTransaction);

		// Allow organizer to create TaskFragment and start/reparent activity to TaskFragment.
		final TaskFragmentCreationParams mockParams = mock(TaskFragmentCreationParams.class);
		doReturn(mOrganizerToken).when(mockParams).getOrganizer();
		mTransaction.createTaskFragment(mockParams);
		mTransaction.createTaskFragment(params);
		mTransaction.startActivityInTaskFragment(
		mFragmentToken, null /* callerToken /, new Intent(), null / activityOptions */);
		mTransaction.reparentActivityToTaskFragment(mFragmentToken, mock(IBinder.class));
		mTransaction.setAdjacentTaskFragments(mFragmentToken, mock(IBinder.class),
		null /* options */);

		// It is expected to fail for the mock TaskFragmentCreationParams. It is ok as we are
		// testing the security check here.
		assertThrows(IllegalArgumentException.class, () -> {
		try {
		mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
		} catch (RemoteException e) {
		fail();

		// Successfully created a TaskFragment.
		final TaskFragment taskFragment = mAtm.mWindowOrganizerController
		.getTaskFragment(fragmentToken);
		assertNotNull(taskFragment);
		assertEquals(activity.getTask(), taskFragment.getTask());
		}
		});

		@Test
		public void testApplyTransaction_createTaskFragment_failForDifferentUid()
		throws RemoteException {
		mController.registerOrganizer(mIOrganizer);
		final ActivityRecord activity = createActivityRecord(mDisplayContent);
		final int uid = Binder.getCallingUid();
		final IBinder fragmentToken = new Binder();
		final TaskFragmentCreationParams params = new TaskFragmentCreationParams.Builder(
		mOrganizerToken, fragmentToken, activity.token).build();
		mOrganizer.applyTransaction(mTransaction);
		mTransaction.createTaskFragment(params);

		// Fail to create TaskFragment when the task uid is different from caller.
		activity.info.applicationInfo.uid = uid;
		activity.getTask().effectiveUid = uid + 1;
		mAtm.getWindowOrganizerController().applyTransaction(mTransaction);

		assertNull(mAtm.mWindowOrganizerController.getTaskFragment(fragmentToken));

		// Fail to create TaskFragment when the task uid is different from owner activity.
		activity.info.applicationInfo.uid = uid + 1;
		activity.getTask().effectiveUid = uid;
		mAtm.getWindowOrganizerController().applyTransaction(mTransaction);

		assertNull(mAtm.mWindowOrganizerController.getTaskFragment(fragmentToken));

		// Successfully created a TaskFragment for same uid.
		activity.info.applicationInfo.uid = uid;
		activity.getTask().effectiveUid = uid;
		mAtm.getWindowOrganizerController().applyTransaction(mTransaction);

		assertNotNull(mAtm.mWindowOrganizerController.getTaskFragment(fragmentToken));
		}

		@Test