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
diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
index 19916bc..9ca01d4 100644
--- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
@@ -1187,7 +1187,8 @@
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 8a9dfc7..7186ed1 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -1015,6 +1015,7 @@
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);
@@ -1034,10 +1035,11 @@
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,
@@ -1210,6 +1212,7 @@
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();
synchronized (mGlobalLock) {
enforceNotIsolatedCaller("startActivityAndWait");
@@ -1237,6 +1240,7 @@
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);
synchronized (mGlobalLock) {
enforceNotIsolatedCaller("startActivityWithConfig");
userId = handleIncomingUser(Binder.getCallingPid(), Binder.getCallingUid(), userId,
@@ -1286,6 +1290,7 @@
Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode,
int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, IBinder permissionToken,
boolean ignoreTargetSecurity, int userId) {
+ assertPackageMatchesCallingUid(callingPackage);
// This is very dangerous -- it allows you to perform a start activity (including
// permission grants) as any app that may launch one of your own activities. So we only
// allow this in two cases:
@@ -1415,6 +1420,7 @@
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");
@@ -1438,6 +1444,7 @@
@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");
@@ -2363,15 +2370,9 @@
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;
@@ -2423,7 +2424,7 @@
/**
* 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) {
@@ -2440,6 +2441,21 @@
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()) {
@@ -2973,6 +2989,7 @@
@Override
public List<IBinder> getAppTasks(String callingPackage) {
int callingUid = Binder.getCallingUid();
+ assertPackageMatchesCallingUid(callingPackage);
long ident = Binder.clearCallingIdentity();
try {
synchronized (mGlobalLock) {
@@ -6193,6 +6210,7 @@
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,
@@ -6208,6 +6226,7 @@
int userId, TaskRecord 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 1eb7455..a8f7768 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.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 @@
// 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 @@
public int startActivity(IBinder whoThread, String callingPackage,
Intent intent, String resolvedType, Bundle bOptions) {
checkCaller();
+ mService.assertPackageMatchesCallingUid(callingPackage);
int callingUser = UserHandle.getCallingUserId();
TaskRecord tr;
