From f9e5c9fe4671813043385406dd9a49b3b9d0d89f Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Tue, 16 Jul 2019 11:24:45 -0700 Subject: [PATCH] Add calling package verification for ATM binder calls In some methods in ActivityTaskManagerService and AppTaskImpl we were not validating if the callingPackage parameter that's passed in from binder actually belongs to the calling uid. But some of our security checks involve retrieving properties of the callingPackage, e.g. ActivityStackSupervisor.getActionRestrictionForCallingPackage(), and apps were able to circumvent these checks. This CL adds checks that provided package name belongs to the calling UID in ATM binder calls, and throws an exception if it doesn't. Bug: 137395936 Test: Build & flash Change-Id: I28608fd31bdbc56b758696a5e3b042c20d59c071 --- .../server/wm/ActivityStackSupervisor.java | 3 +- .../server/wm/ActivityTaskManagerService.java | 36 ++++++++++++++----- .../com/android/server/wm/AppTaskImpl.java | 9 ++--- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java index 2c0f3e65f1980..381aafb8a960d 100644 --- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java @@ -1239,7 +1239,8 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { final PackageInfo packageInfo; try { packageInfo = mService.mContext.getPackageManager() - .getPackageInfo(callingPackage, PackageManager.GET_PERMISSIONS); + .getPackageInfoAsUser(callingPackage, PackageManager.GET_PERMISSIONS, + UserHandle.getUserId(callingUid)); } catch (PackageManager.NameNotFoundException e) { Slog.i(TAG, "Cannot find package info for " + callingPackage); return ACTIVITY_RESTRICTION_NONE; diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 31b7c688d685d..86adb34b24639 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -1043,6 +1043,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { public final int startActivities(IApplicationThread caller, String callingPackage, Intent[] intents, String[] resolvedTypes, IBinder resultTo, Bundle bOptions, int userId) { + assertPackageMatchesCallingUid(callingPackage); final String reason = "startActivities"; enforceNotIsolatedCaller(reason); userId = handleIncomingUser(Binder.getCallingPid(), Binder.getCallingUid(), userId, reason); @@ -1062,10 +1063,11 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { true /*validateIncomingUser*/); } - int startActivityAsUser(IApplicationThread caller, String callingPackage, + private int startActivityAsUser(IApplicationThread caller, String callingPackage, Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode, int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, int userId, boolean validateIncomingUser) { + assertPackageMatchesCallingUid(callingPackage); enforceNotIsolatedCaller("startActivityAsUser"); userId = getActivityStartController().checkTargetUser(userId, validateIncomingUser, @@ -1238,6 +1240,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { public final WaitResult startActivityAndWait(IApplicationThread caller, String callingPackage, Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode, int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, int userId) { + assertPackageMatchesCallingUid(callingPackage); final WaitResult res = new WaitResult(); enforceNotIsolatedCaller("startActivityAndWait"); userId = handleIncomingUser(Binder.getCallingPid(), Binder.getCallingUid(), @@ -1263,6 +1266,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { public final int startActivityWithConfig(IApplicationThread caller, String callingPackage, Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode, int startFlags, Configuration config, Bundle bOptions, int userId) { + assertPackageMatchesCallingUid(callingPackage); enforceNotIsolatedCaller("startActivityWithConfig"); userId = handleIncomingUser(Binder.getCallingPid(), Binder.getCallingUid(), userId, "startActivityWithConfig"); @@ -1447,6 +1451,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { Intent intent, String resolvedType, IVoiceInteractionSession session, IVoiceInteractor interactor, int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, int userId) { + assertPackageMatchesCallingUid(callingPackage); mAmInternal.enforceCallingPermission(BIND_VOICE_INTERACTION, "startVoiceActivity()"); if (session == null || interactor == null) { throw new NullPointerException("null session or interactor"); @@ -1470,6 +1475,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public int startAssistantActivity(String callingPackage, int callingPid, int callingUid, Intent intent, String resolvedType, Bundle bOptions, int userId) { + assertPackageMatchesCallingUid(callingPackage); mAmInternal.enforceCallingPermission(BIND_VOICE_INTERACTION, "startAssistantActivity()"); userId = handleIncomingUser(callingPid, callingUid, userId, "startAssistantActivity"); @@ -2391,15 +2397,9 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { void moveTaskToFrontLocked(@Nullable IApplicationThread appThread, @Nullable String callingPackage, int taskId, int flags, SafeActivityOptions options, boolean fromRecents) { - final int callingPid = Binder.getCallingPid(); final int callingUid = Binder.getCallingUid(); - if (!isSameApp(callingUid, callingPackage)) { - String msg = "Permission Denial: moveTaskToFrontLocked() from pid=" - + Binder.getCallingPid() + " as package " + callingPackage; - Slog.w(TAG, msg); - throw new SecurityException(msg); - } + assertPackageMatchesCallingUid(callingPackage); if (!checkAppSwitchAllowedLocked(callingPid, callingUid, -1, -1, "Task to front")) { SafeActivityOptions.abort(options); return; @@ -2451,7 +2451,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { /** * Return true if callingUid is system, or packageName belongs to that callingUid. */ - boolean isSameApp(int callingUid, @Nullable String packageName) { + private boolean isSameApp(int callingUid, @Nullable String packageName) { try { if (callingUid != 0 && callingUid != SYSTEM_UID) { if (packageName == null) { @@ -2468,6 +2468,21 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { return true; } + /** + * Checks that the provided package name matches the current calling UID, throws a security + * exception if it doesn't. + */ + void assertPackageMatchesCallingUid(@Nullable String packageName) { + final int callingUid = Binder.getCallingUid(); + if (isSameApp(callingUid, packageName)) { + return; + } + final String msg = "Permission Denial: package=" + packageName + + " does not belong to uid=" + callingUid; + Slog.w(TAG, msg); + throw new SecurityException(msg); + } + boolean checkAppSwitchAllowedLocked(int sourcePid, int sourceUid, int callingPid, int callingUid, String name) { if (mAppSwitchesAllowedTime < SystemClock.uptimeMillis()) { @@ -3033,6 +3048,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public List getAppTasks(String callingPackage) { int callingUid = Binder.getCallingUid(); + assertPackageMatchesCallingUid(callingPackage); long ident = Binder.clearCallingIdentity(); try { synchronized (mGlobalLock) { @@ -6332,6 +6348,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { SafeActivityOptions options, int userId, boolean validateIncomingUser, PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart) { + assertPackageMatchesCallingUid(callingPackage); synchronized (mGlobalLock) { return getActivityStartController().startActivitiesInPackage(uid, realCallingPid, realCallingUid, callingPackage, intents, resolvedTypes, resultTo, options, @@ -6347,6 +6364,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { int userId, Task inTask, String reason, boolean validateIncomingUser, PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart) { + assertPackageMatchesCallingUid(callingPackage); synchronized (mGlobalLock) { return getActivityStartController().startActivityInPackage(uid, realCallingPid, realCallingUid, callingPackage, intent, resolvedType, resultTo, resultWho, diff --git a/services/core/java/com/android/server/wm/AppTaskImpl.java b/services/core/java/com/android/server/wm/AppTaskImpl.java index 357f9e5bec6c1..16a75645f9ae5 100644 --- a/services/core/java/com/android/server/wm/AppTaskImpl.java +++ b/services/core/java/com/android/server/wm/AppTaskImpl.java @@ -27,7 +27,6 @@ import android.os.Binder; import android.os.Bundle; import android.os.IBinder; import android.os.UserHandle; -import android.util.Slog; /** * An implementation of IAppTask, that allows an app to manage its own tasks via @@ -97,12 +96,7 @@ class AppTaskImpl extends IAppTask.Stub { // Will bring task to front if it already has a root activity. final int callingPid = Binder.getCallingPid(); final int callingUid = Binder.getCallingUid(); - if (!mService.isSameApp(callingUid, callingPackage)) { - String msg = "Permission Denial: moveToFront() from pid=" - + Binder.getCallingPid() + " as package " + callingPackage; - Slog.w(TAG, msg); - throw new SecurityException(msg); - } + mService.assertPackageMatchesCallingUid(callingPackage); final long origId = Binder.clearCallingIdentity(); try { synchronized (mService.mGlobalLock) { @@ -134,6 +128,7 @@ class AppTaskImpl extends IAppTask.Stub { public int startActivity(IBinder whoThread, String callingPackage, Intent intent, String resolvedType, Bundle bOptions) { checkCaller(); + mService.assertPackageMatchesCallingUid(callingPackage); int callingUser = UserHandle.getCallingUserId(); Task task;