From 1c9bf5cc54d0b32d8f3046c452e710b017c477c0 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Tue, 3 Mar 2020 14:36:21 +0800 Subject: [PATCH] RESTRICT AUTOMERGE Use consistent calling uid and package in navigateUpTo Originally, if the caller of navigateUpTo is alive, even the calling uid is set to the caller who launched the existing destination activity, the uid from caller process has higher priority to replace the given calling uid. So this change doesn't modify the existing behavior if the caller process is valid. Besides, the case of delivering new intent uses the source record as calling identity too, so the case of starting new activity should be consistent. Also forbid attaching null application thread to avoid unexpected state in process record. Bug: 144285917 Test: bit FrameworksServicesTests:ActivityStackTests Test: bit CtsSecurityTestCases:ActivityManagerTest# \ testActivityManager_attachNullApplication Merged-In: I60732f430256d37cb926d08d093581f051c4afed Change-Id: I60732f430256d37cb926d08d093581f051c4afed --- .../android/server/am/ActivityManagerService.java | 5 ++++- .../java/com/android/server/am/ActivityStack.java | 13 +++++++++---- .../com/android/server/am/ActivityStackTests.java | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 348a0376e08cc..e0ac45adb3f41 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -6722,7 +6722,7 @@ public class ActivityManagerService extends IActivityManager.Stub } } - private final boolean attachApplicationLocked(IApplicationThread thread, + private boolean attachApplicationLocked(@NonNull IApplicationThread thread, int pid) { // Find the application record that is being attached... either via @@ -7027,6 +7027,9 @@ public class ActivityManagerService extends IActivityManager.Stub @Override public final void attachApplication(IApplicationThread thread) { + if (thread == null) { + throw new SecurityException("Invalid application interface"); + } synchronized (this) { int callingPid = Binder.getCallingPid(); final long origId = Binder.clearCallingIdentity(); diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java index bde317a163db9..a65a594651666 100644 --- a/services/core/java/com/android/server/am/ActivityStack.java +++ b/services/core/java/com/android/server/am/ActivityStack.java @@ -3853,6 +3853,11 @@ class ActivityStack extends ConfigurationContai final boolean navigateUpToLocked(ActivityRecord srec, Intent destIntent, int resultCode, Intent resultData) { + if (srec.app == null || srec.app.thread == null) { + // Nothing to do if the caller is not attached, because this method should be called + // from an alive activity. + return false; + } final TaskRecord task = srec.getTask(); final ArrayList activities = task.mActivities; final int start = activities.indexOf(srec); @@ -3904,22 +3909,22 @@ class ActivityStack extends ConfigurationContai } if (parent != null && foundParentInTask) { + final int callingUid = srec.info.applicationInfo.uid; final int parentLaunchMode = parent.info.launchMode; final int destIntentFlags = destIntent.getFlags(); if (parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_INSTANCE || parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TASK || parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TOP || (destIntentFlags & Intent.FLAG_ACTIVITY_CLEAR_TOP) != 0) { - parent.deliverNewIntentLocked(srec.info.applicationInfo.uid, destIntent, - srec.packageName); + parent.deliverNewIntentLocked(callingUid, destIntent, srec.packageName); } else { try { ActivityInfo aInfo = AppGlobals.getPackageManager().getActivityInfo( destIntent.getComponent(), 0, srec.userId); int res = mService.mActivityStarter.startActivityLocked(srec.app.thread, destIntent, null /*ephemeralIntent*/, null, aInfo, null /*rInfo*/, null, - null, parent.appToken, null, 0, -1, parent.launchedFromUid, - parent.launchedFromPackage, -1, parent.launchedFromUid, 0, null, + null, parent.appToken, null, 0, -1, callingUid, + srec.packageName, -1, callingUid, 0, null, false, true, null, null, null, "navigateUpTo"); foundParentInTask = res == ActivityManager.START_SUCCESS; } catch (RemoteException e) { diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityStackTests.java b/services/tests/servicestests/src/com/android/server/am/ActivityStackTests.java index 711c36b8b1d49..bdd89da1df887 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityStackTests.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityStackTests.java @@ -17,6 +17,7 @@ package com.android.server.am; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -119,4 +120,17 @@ public class ActivityStackTests extends ActivityTestsBase { assertEquals(ActivityStack.STACK_VISIBLE_ACTIVITY_BEHIND, fullscreenWorkspaceStackId.shouldBeVisible(null /*starting*/)); } + + @Test + public void testNavigateUpTo() { + final ActivityManagerService service = createActivityManagerService(); + final TaskRecord task = createTask(service, testActivityComponent, TEST_STACK_ID); + final ActivityRecord activityRecord = createActivity(service, testActivityComponent, task); + activityRecord.app = new ProcessRecord(null, activityRecord.appInfo, + activityRecord.processName, activityRecord.getUid()); + final ActivityStack testStack = service.mStackSupervisor.getStack(TEST_STACK_ID); + // No-op if the source activity record doesn't have attached process (app.thread == null). + assertFalse(testStack.navigateUpToLocked(activityRecord, activityRecord.intent, + 0 /* resultCode */, null /* resultData */)); + } }