From 744a05e943ec713b6fb85fe8c05f811daaabc335 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Mon, 23 May 2022 19:44:30 +0800 Subject: [PATCH] Fix cross user package visibility leakage for queryInstrumentation Returns false for querying instrumentation APIs if the target package does not installed in the calling user. Also adding a user id parameter to APIs for the system modules to specify the correct user id when querying the instrumentation. NoNonSdkCheck: The equivalent apis are available in PackageManager. Bug: 229684723 Test: atest CrossUserPackageVisibilityTests Change-Id: I0798e543705cb8866231ee376e07b853b419b6f3 --- .../com/android/commands/am/Instrument.java | 3 +- core/java/android/app/ActivityThread.java | 9 +++-- .../app/ApplicationPackageManager.java | 6 +-- .../android/content/pm/IPackageManager.aidl | 10 ++--- .../server/am/ActivityManagerService.java | 11 ++--- .../java/com/android/server/pm/Computer.java | 7 ++-- .../com/android/server/pm/ComputerEngine.java | 40 ++++++++++--------- .../server/pm/IPackageManagerBase.java | 12 +++--- .../server/pm/PackageManagerShellCommand.java | 4 +- .../server/pm/parsing/PackageInfoUtils.java | 10 +++-- 10 files changed, 61 insertions(+), 51 deletions(-) diff --git a/cmds/am/src/com/android/commands/am/Instrument.java b/cmds/am/src/com/android/commands/am/Instrument.java index 7ff4bc4bcf764..2604497dcb63a 100644 --- a/cmds/am/src/com/android/commands/am/Instrument.java +++ b/cmds/am/src/com/android/commands/am/Instrument.java @@ -425,7 +425,8 @@ public class Instrument { if (cn == null) throw new IllegalArgumentException("Bad component name: " + cnArg); return cn; } else { - List infos = mPm.queryInstrumentation(null, 0).getList(); + List infos = mPm.queryInstrumentationAsUser( + null, 0, userId).getList(); final int numInfos = infos == null ? 0: infos.size(); ArrayList cns = new ArrayList<>(); diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 69868ab76c3c1..c87f3c454a093 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -6863,9 +6863,12 @@ public final class ActivityThread extends ClientTransactionHandler private InstrumentationInfo prepareInstrumentation(AppBindData data) { final InstrumentationInfo ii; try { - ii = new ApplicationPackageManager(null, getPackageManager()) - .getInstrumentationInfo(data.instrumentationName, 0); - } catch (PackageManager.NameNotFoundException e) { + ii = getPackageManager().getInstrumentationInfoAsUser(data.instrumentationName, + 0 /* flags */, UserHandle.myUserId()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + if (ii == null) { throw new RuntimeException( "Unable to find instrumentation info for: " + data.instrumentationName); } diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java index e9ebd729ef9fe..fea30278aabe3 100644 --- a/core/java/android/app/ApplicationPackageManager.java +++ b/core/java/android/app/ApplicationPackageManager.java @@ -1695,8 +1695,8 @@ public class ApplicationPackageManager extends PackageManager { ComponentName className, int flags) throws NameNotFoundException { try { - InstrumentationInfo ii = mPM.getInstrumentationInfo( - className, flags); + InstrumentationInfo ii = mPM.getInstrumentationInfoAsUser( + className, flags, getUserId()); if (ii != null) { return ii; } @@ -1713,7 +1713,7 @@ public class ApplicationPackageManager extends PackageManager { String targetPackage, int flags) { try { ParceledListSlice parceledList = - mPM.queryInstrumentation(targetPackage, flags); + mPM.queryInstrumentationAsUser(targetPackage, flags, getUserId()); if (parceledList == null) { return Collections.emptyList(); } diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index 6bb7e3e7d140e..1b1143a992c98 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -201,13 +201,11 @@ interface IPackageManager { ParceledListSlice queryContentProviders( String processName, int uid, long flags, String metaDataKey); - @UnsupportedAppUsage - InstrumentationInfo getInstrumentationInfo( - in ComponentName className, int flags); + InstrumentationInfo getInstrumentationInfoAsUser( + in ComponentName className, int flags, int userId); - @UnsupportedAppUsage(maxTargetSdk = 30, trackingBug = 170729553) - ParceledListSlice queryInstrumentation( - String targetPackage, int flags); + ParceledListSlice queryInstrumentationAsUser( + String targetPackage, int flags, int userId); void finishPackageInstall(int token, boolean didLaunch); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index b0ab89ca15ab8..27f04f9ce38b1 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -14528,6 +14528,7 @@ public class ActivityManagerService extends IActivityManager.Stub if (arguments != null && arguments.hasFileDescriptors()) { throw new IllegalArgumentException("File descriptors passed in Bundle"); } + final IPackageManager pm = AppGlobals.getPackageManager(); synchronized(this) { InstrumentationInfo ii = null; @@ -14536,11 +14537,8 @@ public class ActivityManagerService extends IActivityManager.Stub boolean noRestart = (flags & INSTR_FLAG_NO_RESTART) != 0; try { - ii = mContext.getPackageManager().getInstrumentationInfo( - className, STOCK_PM_FLAGS); - ai = AppGlobals.getPackageManager().getApplicationInfo( - ii.targetPackage, STOCK_PM_FLAGS, userId); - } catch (PackageManager.NameNotFoundException e) { + ii = pm.getInstrumentationInfoAsUser(className, STOCK_PM_FLAGS, userId); + ai = pm.getApplicationInfo(ii.targetPackage, STOCK_PM_FLAGS, userId); } catch (RemoteException e) { } if (ii == null) { @@ -14568,8 +14566,7 @@ public class ActivityManagerService extends IActivityManager.Stub int match = SIGNATURE_NO_MATCH; try { - match = AppGlobals.getPackageManager().checkSignatures( - ii.targetPackage, ii.packageName, userId); + match = pm.checkSignatures(ii.targetPackage, ii.packageName, userId); } catch (RemoteException e) { } if (match < 0 && match != PackageManager.SIGNATURE_FIRST_NOT_SIGNED) { diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java index dfe06cf921df4..33e9b2f27eb2a 100644 --- a/services/core/java/com/android/server/pm/Computer.java +++ b/services/core/java/com/android/server/pm/Computer.java @@ -452,11 +452,12 @@ public interface Computer extends PackageDataSnapshot { @PackageManager.ComponentInfoFlagsBits long flags, @Nullable String metaDataKey); @Nullable - InstrumentationInfo getInstrumentationInfo(@NonNull ComponentName component, int flags); + InstrumentationInfo getInstrumentationInfoAsUser(@NonNull ComponentName component, int flags, + int userId); @NonNull - ParceledListSlice queryInstrumentation( - @NonNull String targetPackage, int flags); + ParceledListSlice queryInstrumentationAsUser( + @NonNull String targetPackage, int flags, int userId); @NonNull List findSharedNonSystemLibraries( diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 45a139dfb28fe..ad0b455454251 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -4927,32 +4927,33 @@ public class ComputerEngine implements Computer { @Nullable @Override - public InstrumentationInfo getInstrumentationInfo(@NonNull ComponentName component, int flags) { + public InstrumentationInfo getInstrumentationInfoAsUser(@NonNull ComponentName component, + int flags, int userId) { final int callingUid = Binder.getCallingUid(); - final int callingUserId = UserHandle.getUserId(callingUid); + enforceCrossUserPermission(callingUid, userId, false /* requireFullPermission */, + false /* checkShell */, "getInstrumentationInfoAsUser"); + if (!mUserManager.exists(userId)) return null; String packageName = component.getPackageName(); final PackageStateInternal ps = mSettings.getPackage(packageName); AndroidPackage pkg = mPackages.get(packageName); if (ps == null || pkg == null) return null; if (shouldFilterApplication( - ps, callingUid, component, TYPE_UNKNOWN, callingUserId)) { + ps, callingUid, component, TYPE_UNKNOWN, userId)) { return null; } final ParsedInstrumentation i = mInstrumentation.get(component); - return PackageInfoUtils.generateInstrumentationInfo(i, pkg, flags, callingUserId, ps); + final PackageUserStateInternal state = ps.getUserStateOrDefault(userId); + return PackageInfoUtils.generateInstrumentationInfo(i, pkg, flags, state, userId, ps); } @NonNull @Override - public ParceledListSlice queryInstrumentation( - @NonNull String targetPackage, int flags) { + public ParceledListSlice queryInstrumentationAsUser( + @NonNull String targetPackage, int flags, int userId) { final int callingUid = Binder.getCallingUid(); - final int callingUserId = UserHandle.getUserId(callingUid); - final PackageStateInternal ps = mSettings.getPackage(targetPackage); - if (shouldFilterApplication(ps, callingUid, callingUserId)) { - return ParceledListSlice.emptyList(); - } - + enforceCrossUserPermission(callingUid, userId, false /* requireFullPermission */, + false /* checkShell */, "queryInstrumentationAsUser"); + if (!mUserManager.exists(userId)) return ParceledListSlice.emptyList(); ArrayList finalList = new ArrayList<>(); final int numInstrumentations = mInstrumentation.size(); @@ -4963,12 +4964,15 @@ public class ComputerEngine implements Computer { String packageName = p.getPackageName(); AndroidPackage pkg = mPackages.get(packageName); PackageStateInternal pkgSetting = getPackageStateInternal(packageName); - if (pkg != null) { - InstrumentationInfo ii = PackageInfoUtils.generateInstrumentationInfo(p, - pkg, flags, callingUserId, pkgSetting); - if (ii != null) { - finalList.add(ii); - } + if (pkg == null || pkgSetting == null + || shouldFilterApplication(pkgSetting, callingUid, userId)) { + continue; + } + final PackageUserStateInternal state = pkgSetting.getUserStateOrDefault(userId); + InstrumentationInfo ii = PackageInfoUtils.generateInstrumentationInfo(p, + pkg, flags, state, userId, pkgSetting); + if (ii != null) { + finalList.add(ii); } } } diff --git a/services/core/java/com/android/server/pm/IPackageManagerBase.java b/services/core/java/com/android/server/pm/IPackageManagerBase.java index 37541312f9b77..c16071dbbb11a 100644 --- a/services/core/java/com/android/server/pm/IPackageManagerBase.java +++ b/services/core/java/com/android/server/pm/IPackageManagerBase.java @@ -533,9 +533,9 @@ public abstract class IPackageManagerBase extends IPackageManager.Stub { @Nullable @Override @Deprecated - public final InstrumentationInfo getInstrumentationInfo(@NonNull ComponentName component, - int flags) { - return snapshot().getInstrumentationInfo(component, flags); + public final InstrumentationInfo getInstrumentationInfoAsUser(@NonNull ComponentName component, + int flags, int userId) { + return snapshot().getInstrumentationInfoAsUser(component, flags, userId); } @Override @@ -1017,9 +1017,9 @@ public abstract class IPackageManagerBase extends IPackageManager.Stub { @NonNull @Override @Deprecated - public final ParceledListSlice queryInstrumentation( - @NonNull String targetPackage, int flags) { - return snapshot().queryInstrumentation(targetPackage, flags); + public final ParceledListSlice queryInstrumentationAsUser( + @NonNull String targetPackage, int flags, int userId) { + return snapshot().queryInstrumentationAsUser(targetPackage, flags, userId); } @Override diff --git a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java index f5981e2ee10d0..5906c3f3f39f3 100644 --- a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java +++ b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java @@ -788,7 +788,9 @@ class PackageManagerShellCommand extends ShellCommand { } final List list = - mInterface.queryInstrumentation(targetPackage, 0 /*flags*/).getList(); + mInterface.queryInstrumentationAsUser( + targetPackage, PackageManager.MATCH_KNOWN_PACKAGES, UserHandle.USER_SYSTEM) + .getList(); // sort by target package Collections.sort(list, new Comparator() { diff --git a/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java b/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java index 4f0f2a85381cb..9c620c4ff3ab1 100644 --- a/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java +++ b/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java @@ -204,7 +204,8 @@ public class PackageInfoUtils { info.instrumentation = new InstrumentationInfo[N]; for (int i = 0; i < N; i++) { info.instrumentation[i] = generateInstrumentationInfo( - pkg.getInstrumentations().get(i), pkg, flags, userId, pkgSetting); + pkg.getInstrumentations().get(i), pkg, flags, state, + userId, pkgSetting); } } } @@ -363,9 +364,12 @@ public class PackageInfoUtils { */ @Nullable public static InstrumentationInfo generateInstrumentationInfo(ParsedInstrumentation i, - AndroidPackage pkg, @PackageManager.ComponentInfoFlagsBits long flags, int userId, - @Nullable PackageStateInternal pkgSetting) { + AndroidPackage pkg, @PackageManager.ComponentInfoFlagsBits long flags, + PackageUserStateInternal state, int userId, @Nullable PackageStateInternal pkgSetting) { if (i == null) return null; + if (!checkUseInstalledOrHidden(pkg, pkgSetting, state, flags)) { + return null; + } InstrumentationInfo info = PackageInfoWithoutStateUtils.generateInstrumentationInfo(i, pkg, flags, userId,