From 342fd8348361a866c4e80eaf7383bdb0bd536736 Mon Sep 17 00:00:00 2001 From: kholoud mohamed Date: Wed, 9 Jun 2021 15:05:18 +0100 Subject: [PATCH] Fix security vulnerability in DPMS Changed DPMS#isPackageAllowedToAccessCalendarForUser to always require INTERACT_ACROSS_USERS/_FULL permission if called for a different uid than the calling uid. Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest Bug: 187043716 Change-Id: I230bbffbdf97c251c8a40add097b3b4254d39452 --- .../app/admin/DevicePolicyManager.java | 4 +++ .../DevicePolicyManagerService.java | 15 ++++++++- .../devicepolicy/DevicePolicyManagerTest.java | 31 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index cb2a45ddc6245..5cf25d59420ea 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -13084,6 +13084,10 @@ public class DevicePolicyManager { * @see #getCrossProfileCalendarPackages(ComponentName) * @hide */ + @RequiresPermission(anyOf = { + permission.INTERACT_ACROSS_USERS_FULL, + permission.INTERACT_ACROSS_USERS + }, conditional = true) public boolean isPackageAllowedToAccessCalendar(@NonNull String packageName) { throwIfParentInstance("isPackageAllowedToAccessCalendar"); if (mService != null) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 9a35738744710..70ba1cbbebc58 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -15992,7 +15992,20 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Preconditions.checkArgumentNonnegative(userHandle, "Invalid userId"); final CallerIdentity caller = getCallerIdentity(); - Preconditions.checkCallAuthorization(hasCrossUsersPermission(caller, userHandle)); + final int packageUid = mInjector.binderWithCleanCallingIdentity(() -> { + try { + return mInjector.getPackageManager().getPackageUidAsUser(packageName, userHandle); + } catch (NameNotFoundException e) { + Slogf.w(LOG_TAG, e, + "Couldn't find package %s in user %d", packageName, userHandle); + return -1; + } + }); + if (caller.getUid() != packageUid) { + Preconditions.checkCallAuthorization( + hasCallingOrSelfPermission(permission.INTERACT_ACROSS_USERS) + || hasCallingOrSelfPermission(permission.INTERACT_ACROSS_USERS_FULL)); + } synchronized (getLockObject()) { if (mInjector.settingsSecureGetIntForUser( diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 654d9fcf9210e..9da2fdc1a0800 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -6521,6 +6521,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().settings.settingsSecureGetIntForUser( Settings.Secure.CROSS_PROFILE_CALENDAR_ENABLED, 0, CALLER_USER_HANDLE)).thenReturn(1); + mContext.permissions.add(permission.INTERACT_ACROSS_USERS); + assertThat(dpm.isPackageAllowedToAccessCalendar("TEST_PACKAGE")).isFalse(); } @@ -6532,6 +6534,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().settings.settingsSecureGetIntForUser( Settings.Secure.CROSS_PROFILE_CALENDAR_ENABLED, 0, CALLER_USER_HANDLE)).thenReturn(0); + mContext.permissions.add(permission.INTERACT_ACROSS_USERS); + assertThat(dpm.isPackageAllowedToAccessCalendar(testPackage)).isFalse(); } @@ -6543,6 +6547,33 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().settings.settingsSecureGetIntForUser( Settings.Secure.CROSS_PROFILE_CALENDAR_ENABLED, 0, CALLER_USER_HANDLE)).thenReturn(1); + mContext.permissions.add(permission.INTERACT_ACROSS_USERS); + + assertThat(dpm.isPackageAllowedToAccessCalendar(testPackage)).isTrue(); + } + + @Test + public void testIsPackageAllowedToAccessCalendar_requiresPermission() { + final String testPackage = "TEST_PACKAGE"; + + assertExpectException(SecurityException.class, /* messageRegex= */ null, + () -> dpm.isPackageAllowedToAccessCalendar(testPackage)); + } + + @Test + public void testIsPackageAllowedToAccessCalendar_samePackageAndSameUser_noPermissionRequired() + throws Exception { + final String testPackage = "TEST_PACKAGE"; + setAsProfileOwner(admin1); + dpm.setCrossProfileCalendarPackages(admin1, null); + when(getServices().settings.settingsSecureGetIntForUser( + Settings.Secure.CROSS_PROFILE_CALENDAR_ENABLED, + 0, CALLER_USER_HANDLE)).thenReturn(1); + doReturn(mContext.binder.callingUid) + .when(getServices().packageManager).getPackageUidAsUser( + eq(testPackage), + anyInt()); + assertThat(dpm.isPackageAllowedToAccessCalendar(testPackage)).isTrue(); }