From 4800557d5b558e653363a89ebd16cc86ea46377e Mon Sep 17 00:00:00 2001 From: Aseem Kumar Date: Mon, 21 Mar 2022 20:35:20 -0700 Subject: [PATCH 01/42] DO NOT MERGE Move accountname and typeName length check from Account.java to AccountManagerService. Bug: 169762606 Test: atest AccountManagerServiceTest Change-Id: I80fabf3a64c55837db98ff316e7e5420129c001b (cherry picked from commit 0adcadb0b28310bac568def4da2cbaf16843bcea) (cherry picked from commit c48f5407d5ae5210b9ee486e362c43a90409faa0) Merged-In: I80fabf3a64c55837db98ff316e7e5420129c001b --- core/java/android/accounts/Account.java | 7 ------ .../accounts/AccountManagerService.java | 12 ++++++++++ .../accounts/AccountManagerServiceTest.java | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/core/java/android/accounts/Account.java b/core/java/android/accounts/Account.java index e6cdcc0ee7420..0d6a07938e959 100644 --- a/core/java/android/accounts/Account.java +++ b/core/java/android/accounts/Account.java @@ -31,7 +31,6 @@ import com.android.internal.annotations.GuardedBy; -import java.util.Objects; import java.util.Set; /** @@ -87,12 +86,6 @@ public Account(String name, String type, String accessId) { if (TextUtils.isEmpty(type)) { throw new IllegalArgumentException("the type must not be empty: " + type); } - if (name.length() > 200) { - throw new IllegalArgumentException("account name is longer than 200 characters"); - } - if (type.length() > 200) { - throw new IllegalArgumentException("account type is longer than 200 characters"); - } this.name = name; this.type = type; this.accessId = accessId; diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index 400b084ee9667..ae2f93449c869 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -1819,6 +1819,14 @@ private boolean addAccountInternal(UserAccounts accounts, Account account, Strin if (account == null) { return false; } + if (account.name != null && account.name.length() > 200) { + Log.w(TAG, "Account cannot be added - Name longer than 200 chars"); + return false; + } + if (account.type != null && account.type.length() > 200) { + Log.w(TAG, "Account cannot be added - Name longer than 200 chars"); + return false; + } if (!isLocalUnlockedUser(accounts.userId)) { Log.w(TAG, "Account " + account.toSafeString() + " cannot be added - user " + accounts.userId + " is locked. callingUid=" + callingUid); @@ -2064,6 +2072,10 @@ public void renameAccount( + ", pid " + Binder.getCallingPid()); } if (accountToRename == null) throw new IllegalArgumentException("account is null"); + if (newName != null && newName.length() > 200) { + Log.e(TAG, "renameAccount failed - account name longer than 200"); + throw new IllegalArgumentException("account name longer than 200"); + } int userId = UserHandle.getCallingUserId(); if (!isAccountManagedByCaller(accountToRename.type, callingUid, userId)) { String msg = String.format( diff --git a/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java index 2690948ca4491..55619bc9c62a2 100644 --- a/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java @@ -36,6 +36,7 @@ import android.accounts.IAccountManagerResponse; import android.app.AppOpsManager; import android.app.INotificationManager; +import android.app.PropertyInvalidatedCache; import android.app.admin.DevicePolicyManager; import android.app.admin.DevicePolicyManagerInternal; import android.content.BroadcastReceiver; @@ -132,6 +133,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { protected void setUp() throws Exception { MockitoAnnotations.initMocks(this); + PropertyInvalidatedCache.disableForTestMode(); + when(mMockPackageManager.checkSignatures(anyInt(), anyInt())) .thenReturn(PackageManager.SIGNATURE_MATCH); final UserInfo ui = new UserInfo(UserHandle.USER_SYSTEM, "user0", 0); @@ -247,6 +250,27 @@ public void testCheckAddAccount() throws Exception { assertEquals(a31, accounts[1]); } + @SmallTest + public void testCheckAddAccountLongName() throws Exception { + unlockSystemUser(); + //test comment + String longString = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaa"; + Account a11 = new Account(longString, AccountManagerServiceTestFixtures.ACCOUNT_TYPE_1); + + mAms.addAccountExplicitly( + a11, /* password= */ "p11", /* extras= */ null, /* callerPackage= */ null); + + String[] list = new String[]{AccountManagerServiceTestFixtures.CALLER_PACKAGE}; + when(mMockPackageManager.getPackagesForUid(anyInt())).thenReturn(list); + Account[] accounts = mAms.getAccountsAsUser(null, + UserHandle.getCallingUserId(), mContext.getOpPackageName()); + assertEquals(0, accounts.length); + } + + @SmallTest public void testPasswords() throws Exception { unlockSystemUser(); From 818899c4a1f0901aac93ea81277822dc04cbf896 Mon Sep 17 00:00:00 2001 From: Thomas Stuart Date: Thu, 23 Jun 2022 14:27:43 -0700 Subject: [PATCH 02/42] switch TelecomManager List getters to ParceledListSlice It was shown that given a large phoneAccountHandles that are over 1 mb, a TransactionTooLarge exception can be silently thrown causing an empty list to be returned. In order to prevent this behavior, all Lists that return a PhoneAccountHandle or PhoneAccount have been switched to ParceledListSlice. bug: 236263294 Test: atest android.telecom.cts.PhoneAccountRegistrarTest #testRegisterPhoneAccountHandleWithFieldOverLimit Change-Id: I025245b2a6f8cfaca86f268851a9d8f0817e07dd Merged-In: I025245b2a6f8cfaca86f268851a9d8f0817e07dd (cherry picked from commit d54a48f42aca2ce06363a2c260cd3a8668e938d8) Merged-In: I025245b2a6f8cfaca86f268851a9d8f0817e07dd --- telecomm/java/android/telecom/TelecomManager.java | 12 ++++++------ .../android/internal/telecom/ITelecomService.aidl | 13 +++++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/telecomm/java/android/telecom/TelecomManager.java b/telecomm/java/android/telecom/TelecomManager.java index 0e7a2695ebf62..25ebb09f0127c 100644 --- a/telecomm/java/android/telecom/TelecomManager.java +++ b/telecomm/java/android/telecom/TelecomManager.java @@ -1274,7 +1274,7 @@ public List getPhoneAccountsSupportingScheme(String uriSchem if (service != null) { try { return service.getPhoneAccountsSupportingScheme(uriScheme, - mContext.getOpPackageName()); + mContext.getOpPackageName()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getPhoneAccountsSupportingScheme", e); } @@ -1316,7 +1316,7 @@ public List getSelfManagedPhoneAccounts() { if (service != null) { try { return service.getSelfManagedPhoneAccounts(mContext.getOpPackageName(), - mContext.getAttributionTag()); + mContext.getAttributionTag()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getSelfManagedPhoneAccounts()", e); } @@ -1342,7 +1342,7 @@ public List getSelfManagedPhoneAccounts() { if (service != null) { try { return service.getCallCapablePhoneAccounts(includeDisabledAccounts, - mContext.getOpPackageName(), mContext.getAttributionTag()); + mContext.getOpPackageName(), mContext.getAttributionTag()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getCallCapablePhoneAccounts(" + includeDisabledAccounts + ")", e); @@ -1366,7 +1366,7 @@ public List getPhoneAccountsForPackage() { ITelecomService service = getTelecomService(); if (service != null) { try { - return service.getPhoneAccountsForPackage(mContext.getPackageName()); + return service.getPhoneAccountsForPackage(mContext.getPackageName()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getPhoneAccountsForPackage", e); } @@ -1426,7 +1426,7 @@ public List getAllPhoneAccounts() { ITelecomService service = getTelecomService(); if (service != null) { try { - return service.getAllPhoneAccounts(); + return service.getAllPhoneAccounts().getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getAllPhoneAccounts", e); } @@ -1445,7 +1445,7 @@ public List getAllPhoneAccountHandles() { ITelecomService service = getTelecomService(); if (service != null) { try { - return service.getAllPhoneAccountHandles(); + return service.getAllPhoneAccountHandles().getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getAllPhoneAccountHandles", e); } diff --git a/telecomm/java/com/android/internal/telecom/ITelecomService.aidl b/telecomm/java/com/android/internal/telecom/ITelecomService.aidl index 6f286d9f3006d..8f409c0972f15 100644 --- a/telecomm/java/com/android/internal/telecom/ITelecomService.aidl +++ b/telecomm/java/com/android/internal/telecom/ITelecomService.aidl @@ -23,6 +23,7 @@ import android.telecom.PhoneAccountHandle; import android.net.Uri; import android.os.Bundle; import android.telecom.PhoneAccount; +import android.content.pm.ParceledListSlice; /** * Interface used to interact with Telecom. Mostly this is used by TelephonyManager for passing @@ -56,25 +57,25 @@ interface ITelecomService { /** * @see TelecomServiceImpl#getCallCapablePhoneAccounts */ - List getCallCapablePhoneAccounts( + ParceledListSlice getCallCapablePhoneAccounts( boolean includeDisabledAccounts, String callingPackage, String callingFeatureId); /** * @see TelecomServiceImpl#getSelfManagedPhoneAccounts */ - List getSelfManagedPhoneAccounts(String callingPackage, + ParceledListSlice getSelfManagedPhoneAccounts(String callingPackage, String callingFeatureId); /** * @see TelecomManager#getPhoneAccountsSupportingScheme */ - List getPhoneAccountsSupportingScheme(in String uriScheme, + ParceledListSlice getPhoneAccountsSupportingScheme(in String uriScheme, String callingPackage); /** * @see TelecomManager#getPhoneAccountsForPackage */ - List getPhoneAccountsForPackage(in String packageName); + ParceledListSlice getPhoneAccountsForPackage(in String packageName); /** * @see TelecomManager#getPhoneAccount @@ -89,12 +90,12 @@ interface ITelecomService { /** * @see TelecomManager#getAllPhoneAccounts */ - List getAllPhoneAccounts(); + ParceledListSlice getAllPhoneAccounts(); /** * @see TelecomManager#getAllPhoneAccountHandles */ - List getAllPhoneAccountHandles(); + ParceledListSlice getAllPhoneAccountHandles(); /** * @see TelecomServiceImpl#getSimCallManager From f7c22a2696cf419c598d1ddbae3d36fdb0466ad7 Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Tue, 2 Aug 2022 03:33:39 +0000 Subject: [PATCH 03/42] [RESTRICT AUTOMERGE] Do not send new Intent to non-exported activity when navigateUpTo The new Intent was delivered to a non-exported activity while #navigateUpTo was called from an Activity of a different uid. Bug: 238605611 Test: atest StartActivityTests Change-Id: I854dd825bfd9a2c08851980d480d1f3a177af6cf Merged-In: I854dd825bfd9a2c08851980d480d1f3a177af6cf (cherry picked from commit 89ebc8c43f7d2aeaee4fdcf667f07aa98404981d) Merged-In: I854dd825bfd9a2c08851980d480d1f3a177af6cf --- .../core/java/com/android/server/wm/Task.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index fd5df6f6c3906..2af7817cacaa0 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -5475,7 +5475,23 @@ boolean navigateUpTo(ActivityRecord srec, Intent destIntent, NeededUriGrants des parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TASK || parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TOP || (destIntentFlags & Intent.FLAG_ACTIVITY_CLEAR_TOP) != 0) { - parent.deliverNewIntentLocked(callingUid, destIntent, destGrants, srec.packageName); + boolean abort; + try { + abort = !mTaskSupervisor.checkStartAnyActivityPermission(destIntent, + parent.info, null /* resultWho */, -1 /* requestCode */, srec.getPid(), + callingUid, srec.info.packageName, null /* callingFeatureId */, + false /* ignoreTargetSecurity */, false /* launchingInTask */, srec.app, + null /* resultRecord */, null /* resultRootTask */); + } catch (SecurityException e) { + abort = true; + } + if (abort) { + android.util.EventLog.writeEvent(0x534e4554, "238605611", callingUid, ""); + foundParentInTask = false; + } else { + parent.deliverNewIntentLocked(callingUid, destIntent, destGrants, + srec.packageName); + } } else { try { ActivityInfo aInfo = AppGlobals.getPackageManager().getActivityInfo( From 0a5fd745866b8fc72c47f5c4ef45635d690c0bcf Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Fri, 12 Aug 2022 11:40:41 -0700 Subject: [PATCH 04/42] Do not send AccessibilityEvent if notification is for different user. Bug: 237540408 Test: BuzzBeepBlinkTest#testA11yCrossUserEventNotSent Change-Id: I62a875e26e214847ec72ce3c41b4f2fa8e597e07 (cherry picked from commit a367c0a16a9070ed6bee3028ac5bbc967773ee8f) Merged-In: I62a875e26e214847ec72ce3c41b4f2fa8e597e07 --- .../notification/NotificationManagerService.java | 3 ++- .../server/notification/BuzzBeepBlinkTest.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 02994406a0d16..5770b1cbbf83f 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -7442,7 +7442,8 @@ int buzzBeepBlinkLocked(NotificationRecord record) { && (record.getSuppressedVisualEffects() & SUPPRESSED_EFFECT_STATUS_BAR) != 0; if (!record.isUpdate && record.getImportance() > IMPORTANCE_MIN - && !suppressedByDnd) { + && !suppressedByDnd + && isNotificationForCurrentUser(record)) { sendAccessibilityEvent(record); sentAccessibilityEvent = true; } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java index d593e8000048f..c1bbad1478725 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java @@ -1299,6 +1299,21 @@ public void testA11yQuietUpdate() throws Exception { verify(mAccessibilityService, times(1)).sendAccessibilityEvent(any(), anyInt()); } + @Test + public void testA11yCrossUserEventNotSent() throws Exception { + final Notification n = new Builder(getContext(), "test") + .setSmallIcon(android.R.drawable.sym_def_app_icon).build(); + int userId = mUser.getIdentifier() + 1; + StatusBarNotification sbn = new StatusBarNotification(mPkg, mPkg, 0, mTag, mUid, + mPid, n, UserHandle.of(userId), null, System.currentTimeMillis()); + NotificationRecord r = new NotificationRecord(getContext(), sbn, + new NotificationChannel("test", "test", IMPORTANCE_HIGH)); + + mService.buzzBeepBlinkLocked(r); + + verify(mAccessibilityService, never()).sendAccessibilityEvent(any(), anyInt()); + } + @Test public void testLightsScreenOn() { mService.mScreenOn = true; From 120f96c6ef965b725598b563a38ca0c46bc93128 Mon Sep 17 00:00:00 2001 From: Ganesh Olekar Date: Fri, 29 Jul 2022 23:44:32 +0000 Subject: [PATCH 05/42] DO NOT MERGE Fix auto-grant of AR runtime permission if device is upgrading from pre-Q Test: Manually install app apks targeting Q and verifying that AR permission is not auto-granted Test: atest ActivityRecognitionPermissionTest Bug: 210065877 Change-Id: I5b2f25218fcbb34a940dfa2ff722cc6595732cfa (cherry picked from commit 23aac9cb8eb4545a79bafef3c14864e0aa59e228) Merged-In: I5b2f25218fcbb34a940dfa2ff722cc6595732cfa --- .../permission/PermissionManagerService.java | 41 +------------------ 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index c1780461b2c1e..d307d06ef106f 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -2774,7 +2774,6 @@ && shouldGrantPermissionByProtectionFlags(pkg, ps, permission, final Permission bp = mRegistry.getPermission(permName); final boolean appSupportsRuntimePermissions = pkg.getTargetSdkVersion() >= Build.VERSION_CODES.M; - String legacyActivityRecognitionPermission = null; if (DEBUG_INSTALL && bp != null) { Log.i(TAG, "Package " + friendlyName @@ -2798,47 +2797,12 @@ && shouldGrantPermissionByProtectionFlags(pkg, ps, permission, // Cache newImplicitPermissions before modifing permissionsState as for the // shared uids the original and new state are the same object if (!origState.hasPermissionState(permName) - && (pkg.getImplicitPermissions().contains(permName) - || (permName.equals(Manifest.permission.ACTIVITY_RECOGNITION)))) { - if (pkg.getImplicitPermissions().contains(permName)) { + && (pkg.getImplicitPermissions().contains(permName))) { // If permName is an implicit permission, try to auto-grant newImplicitPermissions.add(permName); - if (DEBUG_PERMISSIONS) { Slog.i(TAG, permName + " is newly added for " + friendlyName); } - } else { - // Special case for Activity Recognition permission. Even if AR - // permission is not an implicit permission we want to add it to the - // list (try to auto-grant it) if the app was installed on a device - // before AR permission was split, regardless of if the app now requests - // the new AR permission or has updated its target SDK and AR is no - // longer implicit to it. This is a compatibility workaround for apps - // when AR permission was split in Q. - // TODO(zhanghai): This calls into SystemConfig, which generally - // shouldn't cause deadlock, but maybe we should keep a cache of the - // split permission list and just eliminate the possibility. - final List permissionList = - getSplitPermissionInfos(); - int numSplitPerms = permissionList.size(); - for (int splitPermNum = 0; splitPermNum < numSplitPerms; - splitPermNum++) { - PermissionManager.SplitPermissionInfo sp = permissionList.get( - splitPermNum); - String splitPermName = sp.getSplitPermission(); - if (sp.getNewPermissions().contains(permName) - && origState.isPermissionGranted(splitPermName)) { - legacyActivityRecognitionPermission = splitPermName; - newImplicitPermissions.add(permName); - - if (DEBUG_PERMISSIONS) { - Slog.i(TAG, permName + " is newly added for " - + friendlyName); - } - break; - } - } - } } // TODO(b/140256621): The package instant app method has been removed @@ -2969,8 +2933,7 @@ && shouldGrantPermissionByProtectionFlags(pkg, ps, permission, // Hard restricted permissions cannot be held. } else if (!permissionPolicyInitialized || (!hardRestricted || restrictionExempt)) { - if ((origPermState != null && origPermState.isGranted()) - || legacyActivityRecognitionPermission != null) { + if ((origPermState != null && origPermState.isGranted())) { if (!uidState.grantPermission(bp)) { wasChanged = true; } From 82e77e2a6d5992cafa26645d3876c981596f6a60 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Thu, 25 Aug 2022 16:23:12 -0400 Subject: [PATCH 06/42] Check rule package name in ZenModeHelper.addAutomaticRule instead of checking that of the configuration activity, which is potentially spoofable. The package name is verified to be the same app as the caller by NMS. This change removes isSystemRule (called only once) in favor of checking the provided package name directly. Bug: 242537431 Test: ZenModeHelperTest, manual by verifying via provided exploit apk Change-Id: Ic7f350618c26a613df455a4128c9195f4b424a4d (cherry picked from commit 59732d6232d7d82d03897b25be0381c3c510db9b) Merged-In: Ic7f350618c26a613df455a4128c9195f4b424a4d --- .../server/notification/ZenModeHelper.java | 7 +---- .../notification/ZenModeHelperTest.java | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index 5e62496d5cca8..9fd480134ae91 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -310,7 +310,7 @@ public AutomaticZenRule getAutomaticZenRule(String id) { public String addAutomaticZenRule(String pkg, AutomaticZenRule automaticZenRule, String reason) { - if (!isSystemRule(automaticZenRule)) { + if (!ZenModeConfig.SYSTEM_AUTHORITY.equals(pkg)) { PackageItemInfo component = getServiceInfo(automaticZenRule.getOwner()); if (component == null) { component = getActivityInfo(automaticZenRule.getConfigurationActivity()); @@ -566,11 +566,6 @@ protected void updateDefaultZenRules() { } } - private boolean isSystemRule(AutomaticZenRule rule) { - return rule.getOwner() != null - && ZenModeConfig.SYSTEM_AUTHORITY.equals(rule.getOwner().getPackageName()); - } - private ServiceInfo getServiceInfo(ComponentName owner) { Intent queryIntent = new Intent(); queryIntent.setComponent(owner); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java index c0fb39c41676a..5c2730fb1075b 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java @@ -1667,6 +1667,36 @@ public void testAddAutomaticZenRule_beyondSystemLimit_differentComponents() { } } + @Test + public void testAddAutomaticZenRule_claimedSystemOwner() { + // Make sure anything that claims to have a "system" owner but not actually part of the + // system package still gets limited on number of rules + for (int i = 0; i < RULE_LIMIT_PER_PACKAGE; i++) { + ScheduleInfo si = new ScheduleInfo(); + si.startHour = i; + AutomaticZenRule zenRule = new AutomaticZenRule("name" + i, + new ComponentName("android", "ScheduleConditionProvider" + i), + null, // configuration activity + ZenModeConfig.toScheduleConditionId(si), + new ZenPolicy.Builder().build(), + NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); + assertNotNull(id); + } + try { + AutomaticZenRule zenRule = new AutomaticZenRule("name", + new ComponentName("android", "ScheduleConditionProviderFinal"), + null, // configuration activity + ZenModeConfig.toScheduleConditionId(new ScheduleInfo()), + new ZenPolicy.Builder().build(), + NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); + fail("allowed too many rules to be created"); + } catch (IllegalArgumentException e) { + // yay + } + } + @Test public void testAddAutomaticZenRule_CA() { AutomaticZenRule zenRule = new AutomaticZenRule("name", From 4a9fb26eb28ed71182353614bd6c5a1281093853 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Mon, 29 Aug 2022 17:40:14 -0400 Subject: [PATCH 07/42] Trim any long string inputs that come in to AutomaticZenRule This change both prevents any rules from being unable to be written to disk and also avoids risk of running out of memory while handling all the zen rules. Bug: 242703460 Bug: 242703505 Bug: 242703780 Bug: 242704043 Bug: 243794204 Test: cts AutomaticZenRuleTest; atest android.app.AutomaticZenRuleTest; manually confirmed each exploit example either saves the rule successfully with a truncated string (in the case of name & conditionId) or may fail to save the rule at all (if the owner/configactivity is invalid). Additionally ran the memory-exhausting PoC without device crashes. Change-Id: I110172a43f28528dd274b3b346eb29c3796ff2c6 Merged-In: I110172a43f28528dd274b3b346eb29c3796ff2c6 (cherry picked from commit de172ba0d434c940be9e2aad8685719731ab7da2) (cherry picked from commit 19bc2c3559620ed00e448117e65f6b44e6eb6d9b) Merged-In: I110172a43f28528dd274b3b346eb29c3796ff2c6 --- core/java/android/app/AutomaticZenRule.java | 58 +++++-- .../src/android/app/AutomaticZenRuleTest.java | 153 ++++++++++++++++++ 2 files changed, 201 insertions(+), 10 deletions(-) create mode 100644 core/tests/coretests/src/android/app/AutomaticZenRuleTest.java diff --git a/core/java/android/app/AutomaticZenRule.java b/core/java/android/app/AutomaticZenRule.java index 7a806bdf473dd..27a9a27e59a26 100644 --- a/core/java/android/app/AutomaticZenRule.java +++ b/core/java/android/app/AutomaticZenRule.java @@ -47,6 +47,13 @@ public final class AutomaticZenRule implements Parcelable { private boolean mModified = false; private String mPkg; + /** + * The maximum string length for any string contained in this automatic zen rule. This pertains + * both to fields in the rule itself (such as its name) and items with sub-fields. + * @hide + */ + public static final int MAX_STRING_LENGTH = 1000; + /** * Creates an automatic zen rule. * @@ -93,10 +100,10 @@ public AutomaticZenRule(String name, ComponentName owner, Uri conditionId, public AutomaticZenRule(@NonNull String name, @Nullable ComponentName owner, @Nullable ComponentName configurationActivity, @NonNull Uri conditionId, @Nullable ZenPolicy policy, int interruptionFilter, boolean enabled) { - this.name = name; - this.owner = owner; - this.configurationActivity = configurationActivity; - this.conditionId = conditionId; + this.name = getTrimmedString(name); + this.owner = getTrimmedComponentName(owner); + this.configurationActivity = getTrimmedComponentName(configurationActivity); + this.conditionId = getTrimmedUri(conditionId); this.interruptionFilter = interruptionFilter; this.enabled = enabled; this.mZenPolicy = policy; @@ -115,12 +122,12 @@ public AutomaticZenRule(String name, ComponentName owner, ComponentName configur public AutomaticZenRule(Parcel source) { enabled = source.readInt() == ENABLED; if (source.readInt() == ENABLED) { - name = source.readString(); + name = getTrimmedString(source.readString()); } interruptionFilter = source.readInt(); conditionId = source.readParcelable(null); - owner = source.readParcelable(null); - configurationActivity = source.readParcelable(null); + owner = getTrimmedComponentName(source.readParcelable(null)); + configurationActivity = getTrimmedComponentName(source.readParcelable(null)); creationTime = source.readLong(); mZenPolicy = source.readParcelable(null); mModified = source.readInt() == ENABLED; @@ -196,7 +203,7 @@ public long getCreationTime() { * Sets the representation of the state that causes this rule to become active. */ public void setConditionId(Uri conditionId) { - this.conditionId = conditionId; + this.conditionId = getTrimmedUri(conditionId); } /** @@ -211,7 +218,7 @@ public void setInterruptionFilter(@InterruptionFilter int interruptionFilter) { * Sets the name of this rule. */ public void setName(String name) { - this.name = name; + this.name = getTrimmedString(name); } /** @@ -243,7 +250,7 @@ public void setZenPolicy(ZenPolicy zenPolicy) { * that are not backed by {@link android.service.notification.ConditionProviderService}. */ public void setConfigurationActivity(@Nullable ComponentName componentName) { - this.configurationActivity = componentName; + this.configurationActivity = getTrimmedComponentName(componentName); } /** @@ -333,4 +340,35 @@ public AutomaticZenRule[] newArray(int size) { return new AutomaticZenRule[size]; } }; + + /** + * If the package or class name of the provided ComponentName are longer than MAX_STRING_LENGTH, + * return a trimmed version that truncates each of the package and class name at the max length. + */ + private static ComponentName getTrimmedComponentName(ComponentName cn) { + if (cn == null) return null; + return new ComponentName(getTrimmedString(cn.getPackageName()), + getTrimmedString(cn.getClassName())); + } + + /** + * Returns a truncated copy of the string if the string is longer than MAX_STRING_LENGTH. + */ + private static String getTrimmedString(String input) { + if (input != null && input.length() > MAX_STRING_LENGTH) { + return input.substring(0, MAX_STRING_LENGTH); + } + return input; + } + + /** + * Returns a truncated copy of the Uri by trimming the string representation to the maximum + * string length. + */ + private static Uri getTrimmedUri(Uri input) { + if (input != null && input.toString().length() > MAX_STRING_LENGTH) { + return Uri.parse(getTrimmedString(input.toString())); + } + return input; + } } diff --git a/core/tests/coretests/src/android/app/AutomaticZenRuleTest.java b/core/tests/coretests/src/android/app/AutomaticZenRuleTest.java new file mode 100644 index 0000000000000..282fdad294ebc --- /dev/null +++ b/core/tests/coretests/src/android/app/AutomaticZenRuleTest.java @@ -0,0 +1,153 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.fail; + +import android.content.ComponentName; +import android.net.Uri; +import android.os.Parcel; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import com.google.common.base.Strings; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.reflect.Field; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class AutomaticZenRuleTest { + private static final String CLASS = "android.app.AutomaticZenRule"; + + @Test + public void testLongFields_inConstructor() { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + + // test both variants where there's an owner, and where there's a configuration activity + AutomaticZenRule rule1 = new AutomaticZenRule( + longString, // name + new ComponentName("pkg", longString), // owner + null, // configuration activity + longUri, // conditionId + null, // zen policy + 0, // interruption filter + true); // enabled + + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule1.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule1.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule1.getOwner().getClassName().length()); + + AutomaticZenRule rule2 = new AutomaticZenRule( + longString, // name + null, // owner + new ComponentName(longString, "SomeClassName"), // configuration activity + longUri, // conditionId + null, // zen policy + 0, // interruption filter + false); // enabled + + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule2.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule2.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule2.getConfigurationActivity().getPackageName().length()); + } + + @Test + public void testLongFields_inSetters() { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + + AutomaticZenRule rule = new AutomaticZenRule( + "sensible name", + new ComponentName("pkg", "ShortClass"), + null, + Uri.parse("uri://short"), + null, 0, true); + + rule.setName(longString); + rule.setConditionId(longUri); + rule.setConfigurationActivity(new ComponentName(longString, longString)); + + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule.getConfigurationActivity().getPackageName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule.getConfigurationActivity().getClassName().length()); + } + + @Test + public void testLongInputsFromParcel() { + // Create a rule with long fields, set directly via reflection so that we can confirm that + // a rule with too-long fields that comes in via a parcel has its fields truncated directly. + AutomaticZenRule rule = new AutomaticZenRule( + "placeholder", + new ComponentName("place", "holder"), + null, + Uri.parse("uri://placeholder"), + null, 0, true); + + try { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + Field name = Class.forName(CLASS).getDeclaredField("name"); + name.setAccessible(true); + name.set(rule, longString); + Field conditionId = Class.forName(CLASS).getDeclaredField("conditionId"); + conditionId.setAccessible(true); + conditionId.set(rule, longUri); + Field owner = Class.forName(CLASS).getDeclaredField("owner"); + owner.setAccessible(true); + owner.set(rule, new ComponentName(longString, longString)); + Field configActivity = Class.forName(CLASS).getDeclaredField("configurationActivity"); + configActivity.setAccessible(true); + configActivity.set(rule, new ComponentName(longString, longString)); + } catch (NoSuchFieldException e) { + fail(e.toString()); + } catch (ClassNotFoundException e) { + fail(e.toString()); + } catch (IllegalAccessException e) { + fail(e.toString()); + } + + Parcel parcel = Parcel.obtain(); + rule.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + AutomaticZenRule fromParcel = new AutomaticZenRule(parcel); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, fromParcel.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getConfigurationActivity().getPackageName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getConfigurationActivity().getClassName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getOwner().getPackageName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getOwner().getClassName().length()); + } +} From 748ccf1bcae71998c9ca6fffc5afb00e213fa751 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 6 Sep 2022 16:14:16 -0400 Subject: [PATCH 08/42] Fix system zen rules by using owner package name if caller is system Previously were unable to add new zen rules because rules added via the settings pages were getting registered under package "com.android.settings", which then were not considered "system rules". These rules should have package android, so when we can trust the caller (via checking that the caller is system) we should be taking the package name from the owner of the rule. Bug: 245236706 Bug: 242537431 Test: NMSTest; manual Change-Id: Id69b671592396ac3304862dadbe73de328a8e27a Merged-In: Id69b671592396ac3304862dadbe73de328a8e27a (cherry picked from commit 78245566a27882cce59c0d7cd4c60e1604392a3f) Merged-In: Id69b671592396ac3304862dadbe73de328a8e27a --- .../NotificationManagerService.java | 11 +++++- .../NotificationManagerServiceTest.java | 37 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 5770b1cbbf83f..a529843bbfff2 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -4790,7 +4790,16 @@ public String addAutomaticZenRule(AutomaticZenRule automaticZenRule, String pkg) } enforcePolicyAccess(Binder.getCallingUid(), "addAutomaticZenRule"); - return mZenModeHelper.addAutomaticZenRule(pkg, automaticZenRule, + // If the caller is system, take the package name from the rule's owner rather than + // from the caller's package. + String rulePkg = pkg; + if (isCallingUidSystem()) { + if (automaticZenRule.getOwner() != null) { + rulePkg = automaticZenRule.getOwner().getPackageName(); + } + } + + return mZenModeHelper.addAutomaticZenRule(rulePkg, automaticZenRule, "addAutomaticZenRule"); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 9a221a8f2bf9b..861fe404181fe 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -6683,6 +6683,43 @@ public void testAutomaticZenRuleValidation_policyFilterAgreement() throws Except mBinderService.addAutomaticZenRule(rule, mContext.getPackageName()); } + @Test + public void testAddAutomaticZenRule_systemCallTakesPackageFromOwner() throws Exception { + mService.isSystemUid = true; + ZenModeHelper mockZenModeHelper = mock(ZenModeHelper.class); + when(mConditionProviders.isPackageOrComponentAllowed(anyString(), anyInt())) + .thenReturn(true); + mService.setZenHelper(mockZenModeHelper); + ComponentName owner = new ComponentName("android", "ProviderName"); + ZenPolicy zenPolicy = new ZenPolicy.Builder().allowAlarms(true).build(); + boolean isEnabled = true; + AutomaticZenRule rule = new AutomaticZenRule("test", owner, owner, mock(Uri.class), + zenPolicy, NotificationManager.INTERRUPTION_FILTER_PRIORITY, isEnabled); + mBinderService.addAutomaticZenRule(rule, "com.android.settings"); + + // verify that zen mode helper gets passed in a package name of "android" + verify(mockZenModeHelper).addAutomaticZenRule(eq("android"), eq(rule), anyString()); + } + + @Test + public void testAddAutomaticZenRule_nonSystemCallTakesPackageFromArg() throws Exception { + mService.isSystemUid = false; + ZenModeHelper mockZenModeHelper = mock(ZenModeHelper.class); + when(mConditionProviders.isPackageOrComponentAllowed(anyString(), anyInt())) + .thenReturn(true); + mService.setZenHelper(mockZenModeHelper); + ComponentName owner = new ComponentName("android", "ProviderName"); + ZenPolicy zenPolicy = new ZenPolicy.Builder().allowAlarms(true).build(); + boolean isEnabled = true; + AutomaticZenRule rule = new AutomaticZenRule("test", owner, owner, mock(Uri.class), + zenPolicy, NotificationManager.INTERRUPTION_FILTER_PRIORITY, isEnabled); + mBinderService.addAutomaticZenRule(rule, "another.package"); + + // verify that zen mode helper gets passed in the package name from the arg, not the owner + verify(mockZenModeHelper).addAutomaticZenRule( + eq("another.package"), eq(rule), anyString()); + } + @Test public void testAreNotificationsEnabledForPackage_crossUser() throws Exception { try { From 396226ce014ef6694ed5b3108be102914e37568e Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Fri, 14 Jan 2022 13:38:37 -0800 Subject: [PATCH 09/42] Make sure parallel broadcasts enforce excluded permissions Bug: 211029161 Bug: 210118427 Test: atest android.content.cts.ContextWrapperTest#testSendBroadcastRequireNoneOfPermissions_receiverHasExcludedPermissions Merged-In: Ib4fafe2423c7ded1daf1b763f8103601c0e2c852 Change-Id: Ib4fafe2423c7ded1daf1b763f8103601c0e2c852 (cherry picked from commit 0eee4fa476212a35cecee499cfbbfe2d35580bb2) Merged-In: Ib4fafe2423c7ded1daf1b763f8103601c0e2c852 --- .../com/android/server/am/BroadcastQueue.java | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 6daf7099fd446..4a5ae5bc9532c 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -768,6 +768,54 @@ private void deliverToRegisteredReceiverLocked(BroadcastRecord r, } } + // Check that the receiver does *not* have any excluded permissions + if (!skip && r.excludedPermissions != null && r.excludedPermissions.length > 0) { + for (int i = 0; i < r.excludedPermissions.length; i++) { + String excludedPermission = r.excludedPermissions[i]; + final int perm = mService.checkComponentPermission(excludedPermission, + filter.receiverList.pid, filter.receiverList.uid, -1, true); + + int appOp = AppOpsManager.permissionToOpCode(excludedPermission); + if (appOp != AppOpsManager.OP_NONE) { + // When there is an app op associated with the permission, + // skip when both the permission and the app op are + // granted. + if ((perm == PackageManager.PERMISSION_GRANTED) && ( + mService.getAppOpsManager().checkOpNoThrow(appOp, + filter.receiverList.uid, + filter.packageName) + == AppOpsManager.MODE_ALLOWED)) { + Slog.w(TAG, "Appop Denial: receiving " + + r.intent.toString() + + " to " + filter.receiverList.app + + " (pid=" + filter.receiverList.pid + + ", uid=" + filter.receiverList.uid + ")" + + " excludes appop " + AppOpsManager.permissionToOp( + excludedPermission) + + " due to sender " + r.callerPackage + + " (uid " + r.callingUid + ")"); + skip = true; + break; + } + } else { + // When there is no app op associated with the permission, + // skip when permission is granted. + if (perm == PackageManager.PERMISSION_GRANTED) { + Slog.w(TAG, "Permission Denial: receiving " + + r.intent.toString() + + " to " + filter.receiverList.app + + " (pid=" + filter.receiverList.pid + + ", uid=" + filter.receiverList.uid + ")" + + " excludes " + excludedPermission + + " due to sender " + r.callerPackage + + " (uid " + r.callingUid + ")"); + skip = true; + break; + } + } + } + } + // Check that the receiver does *not* belong to any of the excluded packages if (!skip && r.excludedPackages != null && r.excludedPackages.length > 0) { if (ArrayUtils.contains(r.excludedPackages, filter.packageName)) { From b246c9281f7a361bfd3e40c599bf166d24e85dec Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Fri, 1 Jul 2022 09:49:12 -0400 Subject: [PATCH 10/42] Limit the number of concurrently snoozed notifications Test: atest FrameworksUiServicesTests Bug: 234441463 Change-Id: I005b43979d1c708fd505c8b33ae0c8cb03ddbb35 Merged-In: I005b43979d1c708fd505c8b33ae0c8cb03ddbb35 (cherry picked from commit 7c38394ae9c69620499a87e629edae4fe0ac4edc) (cherry picked from commit bc808de2f8a88e76fde0b6d033b4a232aebff8cb) Merged-In: I005b43979d1c708fd505c8b33ae0c8cb03ddbb35 --- .../NotificationManagerService.java | 19 +++-- .../server/notification/SnoozeHelper.java | 11 +++ .../NotificationManagerServiceTest.java | 83 +++++++++++++++++-- .../server/notification/SnoozeHelperTest.java | 17 ++++ 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index a529843bbfff2..0f50900e2bc0f 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -6767,6 +6767,7 @@ public void run() { @GuardedBy("mNotificationLock") void snoozeLocked(NotificationRecord r) { + final List recordsToSnooze = new ArrayList<>(); if (r.getSbn().isGroup()) { final List groupNotifications = findCurrentAndSnoozedGroupNotificationsLocked( @@ -6775,8 +6776,8 @@ void snoozeLocked(NotificationRecord r) { if (r.getNotification().isGroupSummary()) { // snooze all children for (int i = 0; i < groupNotifications.size(); i++) { - if (mKey != groupNotifications.get(i).getKey()) { - snoozeNotificationLocked(groupNotifications.get(i)); + if (!mKey.equals(groupNotifications.get(i).getKey())) { + recordsToSnooze.add(groupNotifications.get(i)); } } } else { @@ -6786,8 +6787,8 @@ void snoozeLocked(NotificationRecord r) { if (groupNotifications.size() == 2) { // snooze summary and the one child for (int i = 0; i < groupNotifications.size(); i++) { - if (mKey != groupNotifications.get(i).getKey()) { - snoozeNotificationLocked(groupNotifications.get(i)); + if (!mKey.equals(groupNotifications.get(i).getKey())) { + recordsToSnooze.add(groupNotifications.get(i)); } } } @@ -6795,7 +6796,15 @@ void snoozeLocked(NotificationRecord r) { } } // snooze the notification - snoozeNotificationLocked(r); + recordsToSnooze.add(r); + + if (mSnoozeHelper.canSnooze(recordsToSnooze.size())) { + for (int i = 0; i < recordsToSnooze.size(); i++) { + snoozeNotificationLocked(recordsToSnooze.get(i)); + } + } else { + Log.w(TAG, "Cannot snooze " + r.getKey() + ": too many snoozed notifications"); + } } diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 4500bbcd250f3..2e08a9cbf844e 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -62,6 +62,8 @@ public class SnoozeHelper { public static final int XML_SNOOZED_NOTIFICATION_VERSION = 1; + static final int CONCURRENT_SNOOZE_LIMIT = 500; + protected static final String XML_TAG_NAME = "snoozed-notifications"; private static final String XML_SNOOZED_NOTIFICATION = "notification"; @@ -134,6 +136,15 @@ void cleanupPersistedContext(String key){ } } + protected boolean canSnooze(int numberToSnooze) { + synchronized (mLock) { + if ((mPackages.size() + numberToSnooze) > CONCURRENT_SNOOZE_LIMIT) { + return false; + } + } + return true; + } + @NonNull protected Long getSnoozeTimeForUnpostedNotification(int userId, String pkg, String key) { Long time = null; diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 861fe404181fe..3d6c6bab243b5 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -2798,20 +2798,81 @@ public void testSameUserSnooze() { any(NotificationManagerService.SnoozeNotificationRunnable.class)); } + @Test + public void testSnoozeRunnable_tooManySnoozed_singleNotification() { + final NotificationRecord notification = generateNotificationRecord( + mTestNotificationChannel, 1, null, true); + mService.addNotification(notification); + + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); + when(mSnoozeHelper.canSnooze(1)).thenReturn(false); + + NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = + mService.new SnoozeNotificationRunnable( + notification.getKey(), 100, null); + snoozeNotificationRunnable.run(); + + verify(mSnoozeHelper, never()).snooze(any(NotificationRecord.class), anyLong()); + assertEquals(1, mService.getNotificationRecordCount()); + } + + @Test + public void testSnoozeRunnable_tooManySnoozed_singleGroupChildNotification() { + final NotificationRecord notification = generateNotificationRecord( + mTestNotificationChannel, 1, "group", true); + final NotificationRecord notificationChild = generateNotificationRecord( + mTestNotificationChannel, 1, "group", false); + mService.addNotification(notification); + mService.addNotification(notificationChild); + + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); + when(mSnoozeHelper.canSnooze(2)).thenReturn(false); + + NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = + mService.new SnoozeNotificationRunnable( + notificationChild.getKey(), 100, null); + snoozeNotificationRunnable.run(); + + verify(mSnoozeHelper, never()).snooze(any(NotificationRecord.class), anyLong()); + assertEquals(2, mService.getNotificationRecordCount()); + } + + @Test + public void testSnoozeRunnable_tooManySnoozed_summaryNotification() { + final NotificationRecord notification = generateNotificationRecord( + mTestNotificationChannel, 1, "group", true); + final NotificationRecord notificationChild = generateNotificationRecord( + mTestNotificationChannel, 12, "group", false); + final NotificationRecord notificationChild2 = generateNotificationRecord( + mTestNotificationChannel, 13, "group", false); + mService.addNotification(notification); + mService.addNotification(notificationChild); + mService.addNotification(notificationChild2); + + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); + when(mSnoozeHelper.canSnooze(3)).thenReturn(false); + + NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = + mService.new SnoozeNotificationRunnable( + notification.getKey(), 100, null); + snoozeNotificationRunnable.run(); + + verify(mSnoozeHelper, never()).snooze(any(NotificationRecord.class), anyLong()); + assertEquals(3, mService.getNotificationRecordCount()); + } + @Test public void testSnoozeRunnable_reSnoozeASingleSnoozedNotification() throws Exception { final NotificationRecord notification = generateNotificationRecord( mTestNotificationChannel, 1, null, true); mService.addNotification(notification); when(mSnoozeHelper.getNotification(any())).thenReturn(notification); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( notification.getKey(), 100, null); snoozeNotificationRunnable.run(); - NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable2 = - mService.new SnoozeNotificationRunnable( - notification.getKey(), 100, null); snoozeNotificationRunnable.run(); // snooze twice @@ -2819,19 +2880,17 @@ mService.new SnoozeNotificationRunnable( } @Test - public void testSnoozeRunnable_reSnoozeASnoozedNotificationWithGroupKey() throws Exception { + public void testSnoozeRunnable_reSnoozeASnoozedNotificationWithGroupKey() { final NotificationRecord notification = generateNotificationRecord( mTestNotificationChannel, 1, "group", true); mService.addNotification(notification); when(mSnoozeHelper.getNotification(any())).thenReturn(notification); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( notification.getKey(), 100, null); snoozeNotificationRunnable.run(); - NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable2 = - mService.new SnoozeNotificationRunnable( - notification.getKey(), 100, null); snoozeNotificationRunnable.run(); // snooze twice @@ -2849,6 +2908,7 @@ public void testSnoozeRunnable_reSnoozeMultipleNotificationsWithGroupKey() throw when(mSnoozeHelper.getNotification(any())).thenReturn(notification); when(mSnoozeHelper.getNotifications( anyString(), anyString(), anyInt())).thenReturn(new ArrayList<>()); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -2858,8 +2918,8 @@ mService.new SnoozeNotificationRunnable( .thenReturn(new ArrayList<>(Arrays.asList(notification, notification2))); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable2 = mService.new SnoozeNotificationRunnable( - notification.getKey(), 100, null); - snoozeNotificationRunnable.run(); + notification2.getKey(), 100, null); + snoozeNotificationRunnable2.run(); // snooze twice verify(mSnoozeHelper, times(4)).snooze(any(NotificationRecord.class), anyLong()); @@ -2873,6 +2933,7 @@ public void testSnoozeRunnable_snoozeNonGrouped() throws Exception { mTestNotificationChannel, 2, "group", false); mService.addNotification(grouped); mService.addNotification(nonGrouped); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -2902,6 +2963,7 @@ public void testSnoozeRunnable_snoozeSummary_withChildren() throws Exception { mService.addNotification(parent); mService.addNotification(child); mService.addNotification(child2); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -2923,6 +2985,7 @@ public void testSnoozeRunnable_snoozeGroupChild_fellowChildren() throws Exceptio mService.addNotification(parent); mService.addNotification(child); mService.addNotification(child2); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -2948,6 +3011,7 @@ public void testSnoozeRunnable_snoozeGroupChild_onlyChildOfSummary() throws Exce mTestNotificationChannel, 2, "group", false); mService.addNotification(parent); mService.addNotification(child); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -2975,6 +3039,7 @@ public void testSnoozeRunnable_snoozeGroupChild_noOthersInGroup() throws Excepti final NotificationRecord child = generateNotificationRecord( mTestNotificationChannel, 2, "group", false); mService.addNotification(child); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( diff --git a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java index 2ae2ef7162a52..8bead57745487 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java @@ -15,6 +15,7 @@ */ package com.android.server.notification; +import static com.android.server.notification.SnoozeHelper.CONCURRENT_SNOOZE_LIMIT; import static com.android.server.notification.SnoozeHelper.EXTRA_KEY; import static junit.framework.Assert.assertEquals; @@ -280,6 +281,22 @@ public void testSnooze() throws Exception { UserHandle.USER_SYSTEM, r.getSbn().getPackageName(), r.getKey())); } + @Test + public void testSnoozeLimit() { + for (int i = 0; i < CONCURRENT_SNOOZE_LIMIT; i++ ) { + NotificationRecord r = getNotificationRecord("pkg", i, i+"", UserHandle.SYSTEM); + + assertTrue("cannot snooze record " + i, mSnoozeHelper.canSnooze(1)); + + if (i % 2 == 0) { + mSnoozeHelper.snooze(r, null); + } else { + mSnoozeHelper.snooze(r, 9000); + } + } + assertFalse(mSnoozeHelper.canSnooze(1)); + } + @Test public void testCancelByApp() throws Exception { NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); From 0cb1977d9c552cb53aeba0d2210882dc06e00bc6 Mon Sep 17 00:00:00 2001 From: Oli Lan Date: Tue, 19 Jul 2022 10:46:00 +0000 Subject: [PATCH 11/42] Revert "Prevent non-admin users from deleting system apps." This reverts commit 6c870e157994519094e9e50ddf93e57a26779e22. Reason for revert: Regression, DELETE_SYSTEM_APP flag no longer works Change-Id: Id3eb9e08a5404e88c39235d0d47337ed41bc6139 Merged-In: I4e959e296cca9bbdfc8fccc5e5e0e654ca524165 (cherry picked from commit d9089fbe06e77f5ea1773f5d69b641a81e0b5832) Merged-In: Id3eb9e08a5404e88c39235d0d47337ed41bc6139 --- .../com/android/server/pm/PackageManagerService.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 66f367cb89652..e69068abfa2f7 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -21750,16 +21750,6 @@ int deletePackageX(String packageName, long versionCode, int userId, int deleteF return PackageManager.DELETE_FAILED_INTERNAL_ERROR; } - if (isSystemApp(uninstalledPs)) { - UserInfo userInfo = mUserManager.getUserInfo(userId); - if (userInfo == null || !userInfo.isAdmin()) { - Slog.w(TAG, "Not removing package " + packageName - + " as only admin user may downgrade system apps"); - EventLog.writeEvent(0x534e4554, "170646036", -1, packageName); - return PackageManager.DELETE_FAILED_USER_RESTRICTED; - } - } - disabledSystemPs = mSettings.getDisabledSystemPkgLPr(packageName); // Static shared libs can be declared by any package, so let us not // allow removing a package if it provides a lib others depend on. From 341facd55eb8350c715365187a988d47c81fb7af Mon Sep 17 00:00:00 2001 From: Oli Lan Date: Wed, 27 Jul 2022 17:17:25 +0000 Subject: [PATCH 12/42] Revert "Prevent exfiltration of system files via user image settings." This reverts commit bfb1cd5fd27c8d854336163540bb5b014ad504d1. Reason for revert: regression if multiple crop system crop handlers are present Change-Id: I570c736ffbd55891bcb2e08110ee4111c5e88d59 Merged-In: Idf1ab60878d619ee30505d71e8afe31d8b0c0ebe (cherry picked from commit 3cfba99d24bc04bc15edd49d1442bee8adb6c171) Merged-In: I570c736ffbd55891bcb2e08110ee4111c5e88d59 --- .../users/EditUserPhotoController.java | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java b/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java index c346142c20f6d..f9584a3e15e9a 100644 --- a/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java +++ b/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java @@ -21,8 +21,6 @@ import android.content.ContentResolver; import android.content.Context; import android.content.Intent; -import android.content.pm.ActivityInfo; -import android.content.pm.PackageManager; import android.database.Cursor; import android.graphics.Bitmap; import android.graphics.Bitmap.Config; @@ -85,7 +83,6 @@ public class EditUserPhotoController { private static final int DEFAULT_PHOTO_SIZE = 500; private static final String IMAGES_DIR = "multi_user"; - private static final String PRE_CROP_PICTURE_FILE_NAME = "PreCropEditUserPhoto.jpg"; private static final String CROP_PICTURE_FILE_NAME = "CropEditUserPhoto.jpg"; private static final String TAKE_PICTURE_FILE_NAME = "TakeEditUserPhoto.jpg"; private static final String NEW_USER_PHOTO_FILE_NAME = "NewUserPhoto.png"; @@ -98,7 +95,6 @@ public class EditUserPhotoController { private final String mFileAuthority; private final File mImagesDir; - private final Uri mPreCropPictureUri; private final Uri mCropPictureUri; private final Uri mTakePictureUri; @@ -114,7 +110,6 @@ public EditUserPhotoController(Activity activity, ActivityStarter activityStarte mImagesDir = new File(activity.getCacheDir(), IMAGES_DIR); mImagesDir.mkdir(); - mPreCropPictureUri = createTempImageUri(activity, PRE_CROP_PICTURE_FILE_NAME, !waiting); mCropPictureUri = createTempImageUri(activity, CROP_PICTURE_FILE_NAME, !waiting); mTakePictureUri = createTempImageUri(activity, TAKE_PICTURE_FILE_NAME, !waiting); mPhotoSize = getPhotoSize(activity); @@ -148,7 +143,7 @@ public boolean onActivityResult(int requestCode, int resultCode, Intent data) { case REQUEST_CODE_CHOOSE_PHOTO: if (mTakePictureUri.equals(pictureUri)) { if (PhotoCapabilityUtils.canCropPhoto(mActivity)) { - cropPhoto(pictureUri); + cropPhoto(); } else { onPhotoNotCropped(pictureUri); } @@ -229,7 +224,7 @@ private void copyAndCropPhoto(final Uri pictureUri) { protected Void doInBackground(Void... params) { final ContentResolver cr = mActivity.getContentResolver(); try (InputStream in = cr.openInputStream(pictureUri); - OutputStream out = cr.openOutputStream(mPreCropPictureUri)) { + OutputStream out = cr.openOutputStream(mTakePictureUri)) { Streams.copy(in, out); } catch (IOException e) { Log.w(TAG, "Failed to copy photo", e); @@ -240,41 +235,28 @@ protected Void doInBackground(Void... params) { @Override protected void onPostExecute(Void result) { if (!mActivity.isFinishing() && !mActivity.isDestroyed()) { - cropPhoto(mPreCropPictureUri); + cropPhoto(); } } }.execute(); } - private void cropPhoto(final Uri pictureUri) { + private void cropPhoto() { // TODO: Use a public intent, when there is one. Intent intent = new Intent("com.android.camera.action.CROP"); - intent.setDataAndType(pictureUri, "image/*"); + intent.setDataAndType(mTakePictureUri, "image/*"); appendOutputExtra(intent, mCropPictureUri); appendCropExtras(intent); - try { - StrictMode.disableDeathOnFileUriExposure(); - if (startSystemActivityForResult(intent, REQUEST_CODE_CROP_PHOTO)) { - return; + if (intent.resolveActivity(mActivity.getPackageManager()) != null) { + try { + StrictMode.disableDeathOnFileUriExposure(); + mActivityStarter.startActivityForResult(intent, REQUEST_CODE_CROP_PHOTO); + } finally { + StrictMode.enableDeathOnFileUriExposure(); } - } finally { - StrictMode.enableDeathOnFileUriExposure(); - } - - onPhotoNotCropped(mTakePictureUri); - - } - - private boolean startSystemActivityForResult(Intent intent, int code) { - ActivityInfo info = intent.resolveActivityInfo(mActivity.getPackageManager(), - PackageManager.MATCH_SYSTEM_ONLY); - if (info == null) { - Log.w(TAG, "No system package activity could be found for code " + code); - return false; + } else { + onPhotoNotCropped(mTakePictureUri); } - intent.setPackage(info.packageName); - mActivityStarter.startActivityForResult(intent, code); - return true; } private void appendOutputExtra(Intent intent, Uri pictureUri) { From 8f30d4cc7266cd8d09c807c54f336abbad0bfd7f Mon Sep 17 00:00:00 2001 From: Oli Lan Date: Tue, 9 Aug 2022 17:48:15 +0100 Subject: [PATCH 13/42] Prevent non-admin users from deleting system apps. This addresses a security issue where the guest user can remove updates for system apps. With this CL, attempts to uninstall/downgrade system apps will fail if attempted by a non-admin user, unless the DELETE_SYSTEM_APP flag is specified. This is a fixed version of ag/17408864, to address b/236578018. Bug: 170646036 Test: manual, try uninstalling system app update as guest Merged-In: I4e959e296cca9bbdfc8fccc5e5e0e654ca524165 Change-Id: I6ecfef50294c9000a6ce539bdec6f372c872a40b (cherry picked from commit fbfa268d47c7915b7a87d3fef22a5b8f3bbabeb7) Merged-In: I6ecfef50294c9000a6ce539bdec6f372c872a40b --- .../com/android/server/pm/PackageManagerService.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index e69068abfa2f7..b4576ec259374 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -21750,6 +21750,17 @@ int deletePackageX(String packageName, long versionCode, int userId, int deleteF return PackageManager.DELETE_FAILED_INTERNAL_ERROR; } + if (isSystemApp(uninstalledPs) + && (deleteFlags & PackageManager.DELETE_SYSTEM_APP) == 0) { + UserInfo userInfo = mUserManager.getUserInfo(userId); + if (userInfo == null || !userInfo.isAdmin()) { + Slog.w(TAG, "Not removing package " + packageName + + " as only admin user may downgrade system apps"); + EventLog.writeEvent(0x534e4554, "170646036", -1, packageName); + return PackageManager.DELETE_FAILED_USER_RESTRICTED; + } + } + disabledSystemPs = mSettings.getDisabledSystemPkgLPr(packageName); // Static shared libs can be declared by any package, so let us not // allow removing a package if it provides a lib others depend on. From 27fcabad6a900020b378cadfae6c9386d700b544 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Fri, 19 Aug 2022 09:54:23 -0400 Subject: [PATCH 14/42] Limit the size of NotificationChannel and NotificationChannelGroup Test: android.app.NotificationChannelGroupTest Test: android.app.NotificationChannelTest Test: cts NotificationChannelTest Test: cts NotificationChannelGroupTest Bug: 241764350 Bug: 241764340 Bug: 241764135 Bug: 242702935 Bug: 242703118 Bug: 242703202 Bug: 242702851 Bug: 242703217 Bug: 242703556 Change-Id: I0925583ab54d6c81c415859618f6b907ab7baada (cherry picked from commit 3850857cb0e7f26702d5bd601731d7290390fa3b) (cherry picked from commit b664159aa809093eec69480e801f367215b78f10) Merged-In: I0925583ab54d6c81c415859618f6b907ab7baada --- .../java/android/app/NotificationChannel.java | 23 ++-- .../android/app/NotificationChannelGroup.java | 10 +- .../app/NotificationChannelGroupTest.java | 73 ++++++++++++ .../android/app/NotificationChannelTest.java | 106 ++++++++++++++++++ 4 files changed, 201 insertions(+), 11 deletions(-) create mode 100644 core/tests/coretests/src/android/app/NotificationChannelGroupTest.java create mode 100644 core/tests/coretests/src/android/app/NotificationChannelTest.java diff --git a/core/java/android/app/NotificationChannel.java b/core/java/android/app/NotificationChannel.java index 208f4cd5ab94d..0e8d4b791ac95 100644 --- a/core/java/android/app/NotificationChannel.java +++ b/core/java/android/app/NotificationChannel.java @@ -124,8 +124,13 @@ public final class NotificationChannel implements Parcelable { /** * The maximum length for text fields in a NotificationChannel. Fields will be truncated at this * limit. + * @hide */ - private static final int MAX_TEXT_LENGTH = 1000; + public static final int MAX_TEXT_LENGTH = 1000; + /** + * @hide + */ + public static final int MAX_VIBRATION_LENGTH = 1000; private static final String TAG_CHANNEL = "channel"; private static final String ATT_NAME = "name"; @@ -290,17 +295,17 @@ public NotificationChannel(String id, CharSequence name, @Importance int importa */ protected NotificationChannel(Parcel in) { if (in.readByte() != 0) { - mId = in.readString(); + mId = getTrimmedString(in.readString()); } else { mId = null; } if (in.readByte() != 0) { - mName = in.readString(); + mName = getTrimmedString(in.readString()); } else { mName = null; } if (in.readByte() != 0) { - mDesc = in.readString(); + mDesc = getTrimmedString(in.readString()); } else { mDesc = null; } @@ -309,18 +314,22 @@ protected NotificationChannel(Parcel in) { mLockscreenVisibility = in.readInt(); if (in.readByte() != 0) { mSound = Uri.CREATOR.createFromParcel(in); + mSound = Uri.parse(getTrimmedString(mSound.toString())); } else { mSound = null; } mLights = in.readByte() != 0; mVibration = in.createLongArray(); + if (mVibration != null && mVibration.length > MAX_VIBRATION_LENGTH) { + mVibration = Arrays.copyOf(mVibration, MAX_VIBRATION_LENGTH); + } mUserLockedFields = in.readInt(); mFgServiceShown = in.readByte() != 0; mVibrationEnabled = in.readByte() != 0; mShowBadge = in.readByte() != 0; mDeleted = in.readByte() != 0; if (in.readByte() != 0) { - mGroup = in.readString(); + mGroup = getTrimmedString(in.readString()); } else { mGroup = null; } @@ -332,8 +341,8 @@ protected NotificationChannel(Parcel in) { mAllowBubbles = in.readInt(); mImportanceLockedByOEM = in.readBoolean(); mOriginalImportance = in.readInt(); - mParentId = in.readString(); - mConversationId = in.readString(); + mParentId = getTrimmedString(in.readString()); + mConversationId = getTrimmedString(in.readString()); mDemoted = in.readBoolean(); mImportantConvo = in.readBoolean(); mDeletedTime = in.readLong(); diff --git a/core/java/android/app/NotificationChannelGroup.java b/core/java/android/app/NotificationChannelGroup.java index cd6df0b231d91..c194a2844bd5e 100644 --- a/core/java/android/app/NotificationChannelGroup.java +++ b/core/java/android/app/NotificationChannelGroup.java @@ -43,8 +43,9 @@ public final class NotificationChannelGroup implements Parcelable { /** * The maximum length for text fields in a NotificationChannelGroup. Fields will be truncated at * this limit. + * @hide */ - private static final int MAX_TEXT_LENGTH = 1000; + public static final int MAX_TEXT_LENGTH = 1000; private static final String TAG_GROUP = "channelGroup"; private static final String ATT_NAME = "name"; @@ -90,13 +91,14 @@ public NotificationChannelGroup(String id, CharSequence name) { */ protected NotificationChannelGroup(Parcel in) { if (in.readByte() != 0) { - mId = in.readString(); + mId = getTrimmedString(in.readString()); } else { mId = null; } mName = TextUtils.CHAR_SEQUENCE_CREATOR.createFromParcel(in); + mName = getTrimmedString(mName.toString()); if (in.readByte() != 0) { - mDescription = in.readString(); + mDescription = getTrimmedString(in.readString()); } else { mDescription = null; } @@ -120,7 +122,7 @@ public void writeToParcel(Parcel dest, int flags) { } else { dest.writeByte((byte) 0); } - TextUtils.writeToParcel(mName, dest, flags); + TextUtils.writeToParcel(mName.toString(), dest, flags); if (mDescription != null) { dest.writeByte((byte) 1); dest.writeString(mDescription); diff --git a/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java b/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java new file mode 100644 index 0000000000000..2a3da05eabb31 --- /dev/null +++ b/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +import static junit.framework.TestCase.assertEquals; + +import android.os.Parcel; +import android.test.AndroidTestCase; + +import androidx.test.filters.SmallTest; +import androidx.test.runner.AndroidJUnit4; + +import com.google.common.base.Strings; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.reflect.Field; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NotificationChannelGroupTest { + private final String CLASS = "android.app.NotificationChannelGroup"; + + @Test + public void testLongStringFields() { + NotificationChannelGroup group = new NotificationChannelGroup("my_group_01", "groupName"); + + try { + String longString = Strings.repeat("A", 65536); + Field mName = Class.forName(CLASS).getDeclaredField("mName"); + mName.setAccessible(true); + mName.set(group, longString); + Field mId = Class.forName(CLASS).getDeclaredField("mId"); + mId.setAccessible(true); + mId.set(group, longString); + Field mDescription = Class.forName(CLASS).getDeclaredField("mDescription"); + mDescription.setAccessible(true); + mDescription.set(group, longString); + } catch (NoSuchFieldException e) { + e.printStackTrace(); + } catch (ClassNotFoundException e) { + e.printStackTrace(); + } catch (IllegalAccessException e) { + e.printStackTrace(); + } + + Parcel parcel = Parcel.obtain(); + group.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + NotificationChannelGroup fromParcel = + NotificationChannelGroup.CREATOR.createFromParcel(parcel); + assertEquals(NotificationChannelGroup.MAX_TEXT_LENGTH, fromParcel.getId().length()); + assertEquals(NotificationChannelGroup.MAX_TEXT_LENGTH, fromParcel.getName().length()); + assertEquals(NotificationChannelGroup.MAX_TEXT_LENGTH, + fromParcel.getDescription().length()); + } +} diff --git a/core/tests/coretests/src/android/app/NotificationChannelTest.java b/core/tests/coretests/src/android/app/NotificationChannelTest.java new file mode 100644 index 0000000000000..647bfe84231d8 --- /dev/null +++ b/core/tests/coretests/src/android/app/NotificationChannelTest.java @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +import static junit.framework.TestCase.assertEquals; + +import android.net.Uri; +import android.os.Parcel; + +import androidx.test.filters.SmallTest; +import androidx.test.runner.AndroidJUnit4; + +import com.google.common.base.Strings; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.reflect.Field; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NotificationChannelTest { + private final String CLASS = "android.app.NotificationChannel"; + + @Test + public void testLongStringFields() { + NotificationChannel channel = new NotificationChannel("id", "name", 3); + + try { + String longString = Strings.repeat("A", 65536); + Field mName = Class.forName(CLASS).getDeclaredField("mName"); + mName.setAccessible(true); + mName.set(channel, longString); + Field mId = Class.forName(CLASS).getDeclaredField("mId"); + mId.setAccessible(true); + mId.set(channel, longString); + Field mDesc = Class.forName(CLASS).getDeclaredField("mDesc"); + mDesc.setAccessible(true); + mDesc.set(channel, longString); + Field mParentId = Class.forName(CLASS).getDeclaredField("mParentId"); + mParentId.setAccessible(true); + mParentId.set(channel, longString); + Field mGroup = Class.forName(CLASS).getDeclaredField("mGroup"); + mGroup.setAccessible(true); + mGroup.set(channel, longString); + Field mConversationId = Class.forName(CLASS).getDeclaredField("mConversationId"); + mConversationId.setAccessible(true); + mConversationId.set(channel, longString); + } catch (NoSuchFieldException e) { + e.printStackTrace(); + } catch (ClassNotFoundException e) { + e.printStackTrace(); + } catch (IllegalAccessException e) { + e.printStackTrace(); + } + + Parcel parcel = Parcel.obtain(); + channel.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + NotificationChannel fromParcel = NotificationChannel.CREATOR.createFromParcel(parcel); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, fromParcel.getId().length()); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, fromParcel.getName().length()); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, + fromParcel.getDescription().length()); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, + fromParcel.getParentChannelId().length()); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, + fromParcel.getGroup().length()); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, + fromParcel.getConversationId().length()); + } + + @Test + public void testLongAlertFields() { + NotificationChannel channel = new NotificationChannel("id", "name", 3); + + channel.setSound(Uri.parse("content://" + Strings.repeat("A",65536)), + Notification.AUDIO_ATTRIBUTES_DEFAULT); + channel.setVibrationPattern(new long[65550/2]); + + Parcel parcel = Parcel.obtain(); + channel.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + NotificationChannel fromParcel = NotificationChannel.CREATOR.createFromParcel(parcel); + assertEquals(NotificationChannel.MAX_VIBRATION_LENGTH, + fromParcel.getVibrationPattern().length); + assertEquals(NotificationChannel.MAX_TEXT_LENGTH, + fromParcel.getSound().toString().length()); + } +} From 4802b6caea8cfcbee721f67c3f5fb2db1628f9ee Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Thu, 1 Sep 2022 10:14:24 -0700 Subject: [PATCH 15/42] Include all enabled services when FEEDBACK_ALL_MASK. Bug: 243849844 Test: m sts; sts-tradefed run sts-dynamic-develop -m CtsAccessibilityTestCases Change-Id: I4f93e06d1066085bd64e8f09882de2f4a72a0633 (cherry picked from commit 2bc4d49c2b0265f5de1c62d1342b1426cc5e1377) Merged-In: I4f93e06d1066085bd64e8f09882de2f4a72a0633 --- .../server/accessibility/AccessibilityManagerService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 04ef10172c88c..42a2bda76c3c6 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -935,7 +935,8 @@ public List getEnabledAccessibilityServiceList(int fee final List result = new ArrayList<>(serviceCount); for (int i = 0; i < serviceCount; ++i) { final AccessibilityServiceConnection service = services.get(i); - if ((service.mFeedbackType & feedbackType) != 0) { + if ((service.mFeedbackType & feedbackType) != 0 + || feedbackType == AccessibilityServiceInfo.FEEDBACK_ALL_MASK) { result.add(service.getServiceInfo()); } } From 6b01b6357c42c79bcacce36b3ef691f3a6624a19 Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Fri, 9 Sep 2022 14:50:31 -0700 Subject: [PATCH 16/42] [pm] forbid deletion of protected packages BUG: 242996180 Test: adb shell pm uninstall --user 0 com.google.android.apps.work.oobconfig Test: Verified with the command above. Before this CL, the package can be deleted. After this CL, the deletion will fail. Change-Id: Iba408e536b340ea5d66ab499442c0c4f828fa36f (cherry picked from commit 15f85c7fa97fe9faa540e6ad9e850990f46a5cca) Merged-In: Iba408e536b340ea5d66ab499442c0c4f828fa36f (cherry picked from commit dba7ceb57ecdf9485bcfe8eb554510ccf9ad773c) Merged-In: Iba408e536b340ea5d66ab499442c0c4f828fa36f --- .../android/server/pm/PackageManagerService.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index b4576ec259374..b868c027259af 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -21447,6 +21447,20 @@ private void deletePackageVersionedInternal(VersionedPackage versionedPackage, final String packageName = versionedPackage.getPackageName(); final long versionCode = versionedPackage.getLongVersionCode(); + + if (mProtectedPackages.isPackageStateProtected(userId, packageName)) { + mHandler.post(() -> { + try { + Slog.w(TAG, "Attempted to delete protected package: " + packageName); + observer.onPackageDeleted(packageName, + PackageManager.DELETE_FAILED_INTERNAL_ERROR, null); + } catch (RemoteException re) { + } + }); + return; + } + + final String internalPackageName; try { From 63995315b1ff28a7ffcfc2e79e8820f697776a60 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 13 Sep 2022 12:53:19 -0400 Subject: [PATCH 17/42] Limit lengths of fields in Condition to a max length. This app-generated input needs to not be too long to avoid errors in the process of writing to disk. Bug: 242846316 Test: cts ConditionTest; atest ConditionTest; manually verified exploit apk is OK Change-Id: Ic2fa8f06cc7a4c1f262115764fbd1be2a226b4b9 Merged-In: Ic2fa8f06cc7a4c1f262115764fbd1be2a226b4b9 (cherry picked from commit 81352c3775949c622441e10b468766441e35edc7) (cherry picked from commit 7059638be9271303e22b7b3e8aa6d58677f6143b) Merged-In: Ic2fa8f06cc7a4c1f262115764fbd1be2a226b4b9 --- .../service/notification/Condition.java | 38 ++++++- .../service/notification/ConditionTest.java | 101 ++++++++++++++++++ 2 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 core/tests/coretests/src/android/service/notification/ConditionTest.java diff --git a/core/java/android/service/notification/Condition.java b/core/java/android/service/notification/Condition.java index 4f324f9e35bff..9a2a86af2b096 100644 --- a/core/java/android/service/notification/Condition.java +++ b/core/java/android/service/notification/Condition.java @@ -90,6 +90,12 @@ public final class Condition implements Parcelable { public final int flags; public final int icon; + /** + * The maximum string length for any string contained in this condition. + * @hide + */ + public static final int MAX_STRING_LENGTH = 1000; + /** * An object representing the current state of a {@link android.app.AutomaticZenRule}. * @param id the {@link android.app.AutomaticZenRule#getConditionId()} of the zen rule @@ -104,16 +110,19 @@ public Condition(Uri id, String summary, String line1, String line2, int icon, if (id == null) throw new IllegalArgumentException("id is required"); if (summary == null) throw new IllegalArgumentException("summary is required"); if (!isValidState(state)) throw new IllegalArgumentException("state is invalid: " + state); - this.id = id; - this.summary = summary; - this.line1 = line1; - this.line2 = line2; + this.id = getTrimmedUri(id); + this.summary = getTrimmedString(summary); + this.line1 = getTrimmedString(line1); + this.line2 = getTrimmedString(line2); this.icon = icon; this.state = state; this.flags = flags; } public Condition(Parcel source) { + // This constructor passes all fields directly into the constructor that takes all the + // fields as arguments; that constructor will trim each of the input strings to + // max length if necessary. this((Uri)source.readParcelable(Condition.class.getClassLoader()), source.readString(), source.readString(), @@ -240,4 +249,25 @@ public Condition[] newArray(int size) { return new Condition[size]; } }; + + /** + * Returns a truncated copy of the string if the string is longer than MAX_STRING_LENGTH. + */ + private static String getTrimmedString(String input) { + if (input != null && input.length() > MAX_STRING_LENGTH) { + return input.substring(0, MAX_STRING_LENGTH); + } + return input; + } + + /** + * Returns a truncated copy of the Uri by trimming the string representation to the maximum + * string length. + */ + private static Uri getTrimmedUri(Uri input) { + if (input != null && input.toString().length() > MAX_STRING_LENGTH) { + return Uri.parse(getTrimmedString(input.toString())); + } + return input; + } } diff --git a/core/tests/coretests/src/android/service/notification/ConditionTest.java b/core/tests/coretests/src/android/service/notification/ConditionTest.java new file mode 100644 index 0000000000000..42629ba41287c --- /dev/null +++ b/core/tests/coretests/src/android/service/notification/ConditionTest.java @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.service.notification; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.fail; + +import android.net.Uri; +import android.os.Parcel; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import com.google.common.base.Strings; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.reflect.Field; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class ConditionTest { + private static final String CLASS = "android.service.notification.Condition"; + + @Test + public void testLongFields_inConstructors() { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + + // Confirm strings are truncated via short constructor + Condition cond1 = new Condition(longUri, longString, Condition.STATE_TRUE); + + assertEquals(Condition.MAX_STRING_LENGTH, cond1.id.toString().length()); + assertEquals(Condition.MAX_STRING_LENGTH, cond1.summary.length()); + + // Confirm strings are truncated via long constructor + Condition cond2 = new Condition(longUri, longString, longString, longString, + -1, Condition.STATE_TRUE, Condition.FLAG_RELEVANT_ALWAYS); + + assertEquals(Condition.MAX_STRING_LENGTH, cond2.id.toString().length()); + assertEquals(Condition.MAX_STRING_LENGTH, cond2.summary.length()); + assertEquals(Condition.MAX_STRING_LENGTH, cond2.line1.length()); + assertEquals(Condition.MAX_STRING_LENGTH, cond2.line2.length()); + } + + @Test + public void testLongFields_viaParcel() { + // Set fields via reflection to force them to be long, then parcel and unparcel to make sure + // it gets truncated upon unparcelling. + Condition cond = new Condition(Uri.parse("uri://placeholder"), "placeholder", + Condition.STATE_TRUE); + + try { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + Field id = Class.forName(CLASS).getDeclaredField("id"); + id.setAccessible(true); + id.set(cond, longUri); + Field summary = Class.forName(CLASS).getDeclaredField("summary"); + summary.setAccessible(true); + summary.set(cond, longString); + Field line1 = Class.forName(CLASS).getDeclaredField("line1"); + line1.setAccessible(true); + line1.set(cond, longString); + Field line2 = Class.forName(CLASS).getDeclaredField("line2"); + line2.setAccessible(true); + line2.set(cond, longString); + } catch (NoSuchFieldException e) { + fail(e.toString()); + } catch (ClassNotFoundException e) { + fail(e.toString()); + } catch (IllegalAccessException e) { + fail(e.toString()); + } + + Parcel parcel = Parcel.obtain(); + cond.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + Condition fromParcel = new Condition(parcel); + assertEquals(Condition.MAX_STRING_LENGTH, fromParcel.id.toString().length()); + assertEquals(Condition.MAX_STRING_LENGTH, fromParcel.summary.length()); + assertEquals(Condition.MAX_STRING_LENGTH, fromParcel.line1.length()); + assertEquals(Condition.MAX_STRING_LENGTH, fromParcel.line2.length()); + } +} From a223859567062d4b2749cb09e1e32b1d24b73219 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Tue, 6 Sep 2022 10:19:06 -0400 Subject: [PATCH 18/42] Fix NPE Test: NotificationChannelGroupTest Test: view notification settings for an app that doesn't use groups Fixes: 244574602 Bug: 241764350 Bug: 241764340 Bug: 241764135 Bug: 242702935 Bug: 242703118 Bug: 242703202 Bug: 242702851 Bug: 242703217 Bug: 242703556 Change-Id: I9c681106f6d645e62b0e44903d40aa523fee0e95 (cherry picked from commit 6f02c07176d0fa4d6985c8f2200ccf49a1657d1c) Merged-In: I9c681106f6d645e62b0e44903d40aa523fee0e95 (cherry picked from commit e51c402650c5e4400108cf63d460ec349577087c) Merged-In: I9c681106f6d645e62b0e44903d40aa523fee0e95 --- .../android/app/NotificationChannelGroup.java | 14 +++++++++++--- .../app/NotificationChannelGroupTest.java | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/core/java/android/app/NotificationChannelGroup.java b/core/java/android/app/NotificationChannelGroup.java index c194a2844bd5e..39e48443f1ffc 100644 --- a/core/java/android/app/NotificationChannelGroup.java +++ b/core/java/android/app/NotificationChannelGroup.java @@ -95,8 +95,11 @@ protected NotificationChannelGroup(Parcel in) { } else { mId = null; } - mName = TextUtils.CHAR_SEQUENCE_CREATOR.createFromParcel(in); - mName = getTrimmedString(mName.toString()); + if (in.readByte() != 0) { + mName = getTrimmedString(in.readString()); + } else { + mName = ""; + } if (in.readByte() != 0) { mDescription = getTrimmedString(in.readString()); } else { @@ -122,7 +125,12 @@ public void writeToParcel(Parcel dest, int flags) { } else { dest.writeByte((byte) 0); } - TextUtils.writeToParcel(mName.toString(), dest, flags); + if (mName != null) { + dest.writeByte((byte) 1); + dest.writeString(mName.toString()); + } else { + dest.writeByte((byte) 0); + } if (mDescription != null) { dest.writeByte((byte) 1); dest.writeString(mDescription); diff --git a/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java b/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java index 2a3da05eabb31..625c66a4c60e9 100644 --- a/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java +++ b/core/tests/coretests/src/android/app/NotificationChannelGroupTest.java @@ -17,9 +17,11 @@ package android.app; import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertTrue; import android.os.Parcel; import android.test.AndroidTestCase; +import android.text.TextUtils; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -70,4 +72,18 @@ public void testLongStringFields() { assertEquals(NotificationChannelGroup.MAX_TEXT_LENGTH, fromParcel.getDescription().length()); } + + @Test + public void testNullableFields() { + NotificationChannelGroup group = new NotificationChannelGroup("my_group_01", null); + + Parcel parcel = Parcel.obtain(); + group.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + NotificationChannelGroup fromParcel = + NotificationChannelGroup.CREATOR.createFromParcel(parcel); + assertEquals(group.getId(), fromParcel.getId()); + assertTrue(TextUtils.isEmpty(fromParcel.getName())); + } } From eb47b2c36a2a1eaeccd4e93c93d0ab9a37c6999a Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 14 Jul 2022 11:25:54 -0700 Subject: [PATCH 19/42] Fix a security issue in app widget service. Bug: 234013191 Test: atest RemoteViewsAdapterTest Change-Id: Icd2eccb7a90124aca18a3dd463c3f79e3a595c20 Merged-In: Icd2eccb7a90124aca18a3dd463c3f79e3a595c20 (cherry picked from commit 263d7d0ba8818c471a27938c4e002bae33569f01) (cherry picked from commit 0ee21ef3e652c78c934d257632a4951bd6d38011) Merged-In: Icd2eccb7a90124aca18a3dd463c3f79e3a595c20 --- core/java/android/appwidget/AppWidgetManager.java | 4 +++- .../com/android/server/appwidget/AppWidgetServiceImpl.java | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/java/android/appwidget/AppWidgetManager.java b/core/java/android/appwidget/AppWidgetManager.java index 18c638112480f..a432b8dec2cbe 100644 --- a/core/java/android/appwidget/AppWidgetManager.java +++ b/core/java/android/appwidget/AppWidgetManager.java @@ -1130,7 +1130,9 @@ public void setBindAppWidgetPermission( * @param intent The intent of the service which will be providing the data to the * RemoteViewsAdapter. * @param connection The callback interface to be notified when a connection is made or lost. - * @param flags Flags used for binding to the service + * @param flags Flags used for binding to the service. Currently only + * {@link Context#BIND_AUTO_CREATE} and + * {@link Context#BIND_FOREGROUND_SERVICE_WHILE_AWAKE} are supported. * * @see Context#getServiceDispatcher(ServiceConnection, Handler, int) * @hide diff --git a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java index d2499313e89db..8abf712a090c7 100644 --- a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java +++ b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java @@ -1199,11 +1199,12 @@ public boolean bindRemoteViewsService(String callingPackage, int appWidgetId, In try { // Ask ActivityManager to bind it. Notice that we are binding the service with the // caller app instead of DevicePolicyManagerService. - if(ActivityManager.getService().bindService( + if (ActivityManager.getService().bindService( caller, activtiyToken, intent, intent.resolveTypeIfNeeded(mContext.getContentResolver()), - connection, flags, mContext.getOpPackageName(), - widget.provider.getUserId()) != 0) { + connection, flags & (Context.BIND_AUTO_CREATE + | Context.BIND_FOREGROUND_SERVICE_WHILE_AWAKE), + mContext.getOpPackageName(), widget.provider.getUserId()) != 0) { // Add it to the mapping of RemoteViewsService to appWidgetIds so that we // can determine when we can call back to the RemoteViewsService later to From 6171c9195a05196d71e51ec67834edd5616480fe Mon Sep 17 00:00:00 2001 From: Jeff Chang Date: Wed, 14 Sep 2022 17:32:53 +0800 Subject: [PATCH 20/42] [RESTRICT AUTOMERGE] Allow activity to be reparent while allowTaskReparenting is applied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Any malicious application could hijack tasks by android:allowTaskReparenting. This vulnerability can perform UI spoofing or spying on user’s activities. This CL only allows activities to be reparent while android:allowTaskReparenting is applied and the affinity of activity is same with the target task. Bug: 240663194 Test: atest IntentTests Change-Id: I73abb9ec05af95bc14f887ae825a9ada9600f771 (cherry picked from commit 7af50c4d5f0354438872167b0e446930caca9deb) Merged-In: I73abb9ec05af95bc14f887ae825a9ada9600f771 --- .../core/java/com/android/server/wm/ResetTargetTaskHelper.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/wm/ResetTargetTaskHelper.java b/services/core/java/com/android/server/wm/ResetTargetTaskHelper.java index 2626b87b93e5e..7e479679e1a2e 100644 --- a/services/core/java/com/android/server/wm/ResetTargetTaskHelper.java +++ b/services/core/java/com/android/server/wm/ResetTargetTaskHelper.java @@ -146,15 +146,16 @@ private boolean processActivity(ActivityRecord r, boolean isTargetTask) { return false; } else { - mResultActivities.add(r); if (r.resultTo != null) { // If this activity is sending a reply to a previous activity, we can't do // anything with it now until we reach the start of the reply chain. // NOTE: that we are assuming the result is always to the previous activity, // which is almost always the case but we really shouldn't count on. + mResultActivities.add(r); return false; } else if (mTargetTaskFound && allowTaskReparenting && mTargetTask.affinity != null && mTargetTask.affinity.equals(r.taskAffinity)) { + mResultActivities.add(r); // This activity has an affinity for our task. Either remove it if we are // clearing or move it over to our task. Note that we currently punt on the case // where we are resetting a task that is not at the top but who has activities From e4385e0c952b9f48ff277634760379ae0b65968b Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Wed, 30 Mar 2022 16:01:10 -0400 Subject: [PATCH 21/42] [DO NOT MERGE] Update window with FLAG_SECURE when bouncer is showing This will prevent bouncer interactions from showing up in screenrecords or screenshots. Fixes: 215005011 Test: atest NotificationShadeWindowControllerImpl && take screenshot with bouncer up Change-Id: I3f59df865dc2dd13d4b9ac54bb2dacb7b23f0aa1 Merged-In: I3f59df865dc2dd13d4b9ac54bb2dacb7b23f0aa1 (cherry picked from commit 6888543d3a6cde5a9e16f9accb0a4256152aba2d) (cherry picked from commit 18ddad1f5a3d9592e063c3d3a70278bccc2e08e5) Merged-In: I3f59df865dc2dd13d4b9ac54bb2dacb7b23f0aa1 --- ...NotificationShadeWindowControllerImpl.java | 11 +++++++++ ...ficationShadeWindowControllerImplTest.java | 24 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImpl.java index 8c76a1bf4f83d..787f9852c1f18 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImpl.java @@ -28,6 +28,7 @@ import android.content.pm.ActivityInfo; import android.graphics.PixelFormat; import android.os.Binder; +import android.os.Build; import android.os.RemoteException; import android.os.Trace; import android.util.Log; @@ -325,6 +326,16 @@ private void applyKeyguardFlags(State state) { Trace.setCounter("display_max_refresh_rate", (long) mLpChanged.preferredMaxDisplayRefreshRate); } + + if (state.mBouncerShowing && !isDebuggable()) { + mLpChanged.flags |= LayoutParams.FLAG_SECURE; + } else { + mLpChanged.flags &= ~LayoutParams.FLAG_SECURE; + } + } + + protected boolean isDebuggable() { + return Build.IS_DEBUGGABLE; } private void adjustScreenOrientation(State state) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImplTest.java index cb468108880a3..bc3f228015b39 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationShadeWindowControllerImplTest.java @@ -18,6 +18,7 @@ import static android.view.WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM; import static android.view.WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE; +import static android.view.WindowManager.LayoutParams.FLAG_SECURE; import static android.view.WindowManager.LayoutParams.FLAG_SHOW_WALLPAPER; import static com.google.common.truth.Truth.assertThat; @@ -90,7 +91,12 @@ public void setUp() { mWindowManager, mActivityManager, mDozeParameters, mStatusBarStateController, mConfigurationController, mKeyguardViewMediator, mKeyguardBypassController, mColorExtractor, mDumpManager, mKeyguardStateController, - mUnlockedScreenOffAnimationController, mAuthController); + mUnlockedScreenOffAnimationController, mAuthController) { + @Override + protected boolean isDebuggable() { + return false; + } + }; mNotificationShadeWindowController.setScrimsVisibilityListener((visibility) -> {}); mNotificationShadeWindowController.setNotificationShadeView(mNotificationShadeWindowView); @@ -239,4 +245,20 @@ public void batchApplyWindowLayoutParams_doesNotDispatchEvents() { }); verify(mWindowManager).updateViewLayout(any(), any()); } + + @Test + public void setKeyguardShowing_enablesSecureFlag() { + mNotificationShadeWindowController.setBouncerShowing(true); + + verify(mWindowManager).updateViewLayout(any(), mLayoutParameters.capture()); + assertThat((mLayoutParameters.getValue().flags & FLAG_SECURE) != 0).isTrue(); + } + + @Test + public void setKeyguardNotShowing_disablesSecureFlag() { + mNotificationShadeWindowController.setBouncerShowing(false); + + verify(mWindowManager).updateViewLayout(any(), mLayoutParameters.capture()); + assertThat((mLayoutParameters.getValue().flags & FLAG_SECURE) == 0).isTrue(); + } } From d0a144e7a9212ba6252791a41e8b951bdabb6665 Mon Sep 17 00:00:00 2001 From: Oli Lan Date: Fri, 26 Aug 2022 17:35:21 +0100 Subject: [PATCH 22/42] Prevent exfiltration of system files via avatar picker. This adds mitigations to prevent system files being exfiltrated via the settings content provider when a content URI is provided as a chosen user image. The mitigations are: 1) Copy the image to a new URI rather than the existing takePictureUri prior to cropping. 2) Only allow a system handler to respond to the CROP intent. This is a fixed version of ag/17071224, to address b/239513606. Bug: 187702830 Test: build and check functionality Change-Id: Ie352d07bbcfc7e0b0a1db1dbe3fd43085e0ecbb6 Merged-In: Idf1ab60878d619ee30505d71e8afe31d8b0c0ebe (cherry picked from commit 1b48ca6b7f44bacdb7b9469bfaf08fe4881ea0ae) Merged-In: Ie352d07bbcfc7e0b0a1db1dbe3fd43085e0ecbb6 --- .../users/EditUserPhotoController.java | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java b/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java index f9584a3e15e9a..0c6cd048619cb 100644 --- a/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java +++ b/packages/SettingsLib/src/com/android/settingslib/users/EditUserPhotoController.java @@ -21,6 +21,8 @@ import android.content.ContentResolver; import android.content.Context; import android.content.Intent; +import android.content.pm.PackageManager; +import android.content.pm.ResolveInfo; import android.database.Cursor; import android.graphics.Bitmap; import android.graphics.Bitmap.Config; @@ -83,6 +85,7 @@ public class EditUserPhotoController { private static final int DEFAULT_PHOTO_SIZE = 500; private static final String IMAGES_DIR = "multi_user"; + private static final String PRE_CROP_PICTURE_FILE_NAME = "PreCropEditUserPhoto.jpg"; private static final String CROP_PICTURE_FILE_NAME = "CropEditUserPhoto.jpg"; private static final String TAKE_PICTURE_FILE_NAME = "TakeEditUserPhoto.jpg"; private static final String NEW_USER_PHOTO_FILE_NAME = "NewUserPhoto.png"; @@ -95,6 +98,7 @@ public class EditUserPhotoController { private final String mFileAuthority; private final File mImagesDir; + private final Uri mPreCropPictureUri; private final Uri mCropPictureUri; private final Uri mTakePictureUri; @@ -110,6 +114,7 @@ public EditUserPhotoController(Activity activity, ActivityStarter activityStarte mImagesDir = new File(activity.getCacheDir(), IMAGES_DIR); mImagesDir.mkdir(); + mPreCropPictureUri = createTempImageUri(activity, PRE_CROP_PICTURE_FILE_NAME, !waiting); mCropPictureUri = createTempImageUri(activity, CROP_PICTURE_FILE_NAME, !waiting); mTakePictureUri = createTempImageUri(activity, TAKE_PICTURE_FILE_NAME, !waiting); mPhotoSize = getPhotoSize(activity); @@ -143,7 +148,7 @@ public boolean onActivityResult(int requestCode, int resultCode, Intent data) { case REQUEST_CODE_CHOOSE_PHOTO: if (mTakePictureUri.equals(pictureUri)) { if (PhotoCapabilityUtils.canCropPhoto(mActivity)) { - cropPhoto(); + cropPhoto(pictureUri); } else { onPhotoNotCropped(pictureUri); } @@ -224,7 +229,7 @@ private void copyAndCropPhoto(final Uri pictureUri) { protected Void doInBackground(Void... params) { final ContentResolver cr = mActivity.getContentResolver(); try (InputStream in = cr.openInputStream(pictureUri); - OutputStream out = cr.openOutputStream(mTakePictureUri)) { + OutputStream out = cr.openOutputStream(mPreCropPictureUri)) { Streams.copy(in, out); } catch (IOException e) { Log.w(TAG, "Failed to copy photo", e); @@ -235,28 +240,41 @@ protected Void doInBackground(Void... params) { @Override protected void onPostExecute(Void result) { if (!mActivity.isFinishing() && !mActivity.isDestroyed()) { - cropPhoto(); + cropPhoto(mPreCropPictureUri); } } }.execute(); } - private void cropPhoto() { + private void cropPhoto(final Uri pictureUri) { // TODO: Use a public intent, when there is one. Intent intent = new Intent("com.android.camera.action.CROP"); - intent.setDataAndType(mTakePictureUri, "image/*"); + intent.setDataAndType(pictureUri, "image/*"); appendOutputExtra(intent, mCropPictureUri); appendCropExtras(intent); - if (intent.resolveActivity(mActivity.getPackageManager()) != null) { - try { - StrictMode.disableDeathOnFileUriExposure(); - mActivityStarter.startActivityForResult(intent, REQUEST_CODE_CROP_PHOTO); - } finally { - StrictMode.enableDeathOnFileUriExposure(); + try { + StrictMode.disableDeathOnFileUriExposure(); + if (startSystemActivityForResult(intent, REQUEST_CODE_CROP_PHOTO)) { + return; } - } else { - onPhotoNotCropped(mTakePictureUri); + } finally { + StrictMode.enableDeathOnFileUriExposure(); + } + + onPhotoNotCropped(mTakePictureUri); + + } + + private boolean startSystemActivityForResult(Intent intent, int code) { + List resolveInfos = mActivity.getPackageManager() + .queryIntentActivities(intent, PackageManager.MATCH_SYSTEM_ONLY); + if (resolveInfos.isEmpty()) { + Log.w(TAG, "No system package activity could be found for code " + code); + return false; } + intent.setPackage(resolveInfos.get(0).activityInfo.packageName); + mActivityStarter.startActivityForResult(intent, code); + return true; } private void appendOutputExtra(Intent intent, Uri pictureUri) { From eeec6a90b78df8e58e36941a230ee914c7a9f925 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Wed, 21 Sep 2022 23:40:21 +0000 Subject: [PATCH 23/42] [Do Not Merge] Ignore malformed shortcuts After an app publishes a shortcut that contains malformed intent, the system can be stuck in boot-loop due to uncaught exception caused by parsing the malformed intent. This CL ignores that particular malformed entry. Since shortcuts are constantly writes back into the xml from system memory, the malformed entry will be removed from the xml the next time system persists shortcuts from memory to file system. Bug: 246540168 Change-Id: Ibbfd0891eabdce72f76571798382fe949d8f453d Test: manual (cherry picked from commit 36338a315218221e51c24a42e44c4f743d416f82) Merged-In: Ibbfd0891eabdce72f76571798382fe949d8f453d --- .../java/com/android/server/pm/ShortcutPackage.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index 89161f3ad3ce6..c55a672aafb79 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -1968,10 +1968,15 @@ public static ShortcutPackage loadFromXml(ShortcutService s, ShortcutUser shortc continue; case TAG_SHORTCUT: - final ShortcutInfo si = parseShortcut(parser, packageName, - shortcutUser.getUserId(), fromBackup); - // Don't use addShortcut(), we don't need to save the icon. - ret.mShortcuts.put(si.getId(), si); + try { + final ShortcutInfo si = parseShortcut(parser, packageName, + shortcutUser.getUserId(), fromBackup); + // Don't use addShortcut(), we don't need to save the icon. + ret.mShortcuts.put(si.getId(), si); + } catch (Exception e) { + // b/246540168 malformed shortcuts should be ignored + Slog.e(TAG, "Failed parsing shortcut.", e); + } continue; case TAG_SHARE_TARGET: ret.mShareTargets.add(ShareTargetInfo.loadFromXml(parser)); From f6c343db5e65572cb370e3b298b6805474e4dd60 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 31 Aug 2022 12:58:41 -0400 Subject: [PATCH 24/42] Lower per-app notificationchannel limit Test: PreferencesHelperTest Bug: 240422263 Change-Id: I8c12e3fc73e4a88842af275feaf2acffcced0402 (cherry picked from commit f528b337dd48b7e8071269e07e610bd4a3668c75) Merged-In: I8c12e3fc73e4a88842af275feaf2acffcced0402 (cherry picked from commit 36acdd675890296151b7a99e05fa7436d59a289b) Merged-In: I8c12e3fc73e4a88842af275feaf2acffcced0402 --- .../java/com/android/server/notification/PreferencesHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 185c0231b534e..72b75f3e80d15 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -97,7 +97,7 @@ public class PreferencesHelper implements RankingConfig { private static final String NON_BLOCKABLE_CHANNEL_DELIM = ":"; @VisibleForTesting - static final int NOTIFICATION_CHANNEL_COUNT_LIMIT = 50000; + static final int NOTIFICATION_CHANNEL_COUNT_LIMIT = 5000; private static final int NOTIFICATION_PREFERENCES_PULL_LIMIT = 1000; private static final int NOTIFICATION_CHANNEL_PULL_LIMIT = 2000; From 3021bb355cc31e89af35e1ae2d99a8e52583cb30 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Mon, 26 Sep 2022 21:35:26 +0800 Subject: [PATCH 25/42] [DO NOT MERGE] Fix permanent denial of service via setComponentEnabledSetting Do not update invalid component enabled settings to prevent the malicious apps from exhausting system server memory. Bug: 240936919 Test: atest android.security.cts.PackageManagerTest Change-Id: I08165337895e89f13a2b9fcce1201cba9ad13d7d (cherry picked from commit 24473590373902db492de502c7c557ef5ead485f) Merged-In: I08165337895e89f13a2b9fcce1201cba9ad13d7d --- .../core/java/com/android/server/pm/PackageManagerService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index b868c027259af..4c34262bdbeb9 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -24261,6 +24261,9 @@ && checkPermission(Manifest.permission.SUSPEND_APPS, packageName, userId) } else { Slog.w(TAG, "Failed setComponentEnabledSetting: component class " + className + " does not exist in " + packageName); + // Safetynet logging for b/240936919 + EventLog.writeEvent(0x534e4554, "240936919", callingUid); + return; } } switch (newState) { From 6fae9473708e9d281f8a197a3886ab71e8a61d63 Mon Sep 17 00:00:00 2001 From: Hao Ke Date: Tue, 4 Oct 2022 19:43:58 +0000 Subject: [PATCH 26/42] Add safety checks on KEY_INTENT mismatch. For many years, Parcel mismatch typed exploits has been using the AccoungManagerService's passing of KEY_INTENT workflow, as a foothold of launching arbitrary intents. We are adding an extra check on the service side to simulate the final deserialization of the KEY_INTENT value, to make sure the client side won't get a mismatched KEY_INTENT value. Bug: 250588548 Bug: 240138294 Test: atest CtsAccountManagerTestCases Test: local test, also see b/250588548 Change-Id: I433e34f6e21ce15c89825044a15b1dec46bb25cc (cherry picked from commit eb9a0566a583fa13f8aff671c41f78a9e33eab82) Merged-In: I433e34f6e21ce15c89825044a15b1dec46bb25cc --- .../accounts/AccountManagerService.java | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index ae2f93449c869..a8672ff38e9d4 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -88,6 +88,7 @@ import android.os.UserManager; import android.stats.devicepolicy.DevicePolicyEnums; import android.text.TextUtils; +import android.util.EventLog; import android.util.Log; import android.util.Pair; import android.util.Slog; @@ -3097,7 +3098,7 @@ public void onResult(Bundle result) { */ if (!checkKeyIntent( Binder.getCallingUid(), - intent)) { + result)) { onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, "invalid intent in bundle returned"); return; @@ -3516,7 +3517,7 @@ public void onResult(Bundle result) { && (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) { if (!checkKeyIntent( Binder.getCallingUid(), - intent)) { + result)) { onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, "invalid intent in bundle returned"); return; @@ -4867,7 +4868,13 @@ IAccountManagerResponse getResponseAndClose() { * into launching arbitrary intents on the device via by tricking to click authenticator * supplied entries in the system Settings app. */ - protected boolean checkKeyIntent(int authUid, Intent intent) { + protected boolean checkKeyIntent(int authUid, Bundle bundle) { + if (!checkKeyIntentParceledCorrectly(bundle)) { + EventLog.writeEvent(0x534e4554, "250588548", authUid, ""); + return false; + } + + Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT); // Explicitly set an empty ClipData to ensure that we don't offer to // promote any Uris contained inside for granting purposes if (intent.getClipData() == null) { @@ -4904,6 +4911,25 @@ protected boolean checkKeyIntent(int authUid, Intent intent) { } } + /** + * Simulate the client side's deserialization of KEY_INTENT value, to make sure they don't + * violate our security policy. + * + * In particular we want to make sure the Authenticator doesn't trick users + * into launching arbitrary intents on the device via exploiting any other Parcel read/write + * mismatch problems. + */ + private boolean checkKeyIntentParceledCorrectly(Bundle bundle) { + Parcel p = Parcel.obtain(); + p.writeBundle(bundle); + p.setDataPosition(0); + Bundle simulateBundle = p.readBundle(); + p.recycle(); + Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT); + Intent simulateIntent = simulateBundle.getParcelable(AccountManager.KEY_INTENT); + return (intent.filterEquals(simulateIntent)); + } + private boolean isExportedSystemActivity(ActivityInfo activityInfo) { String className = activityInfo.name; return "android".equals(activityInfo.packageName) && @@ -5050,7 +5076,7 @@ public void onResult(Bundle result) { && (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) { if (!checkKeyIntent( Binder.getCallingUid(), - intent)) { + result)) { onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, "invalid intent in bundle returned"); return; From 824d2850527432586099cf3b699d8786bef70d30 Mon Sep 17 00:00:00 2001 From: Oli Lan Date: Fri, 19 Aug 2022 17:08:13 +0100 Subject: [PATCH 27/42] Validate package name passed to setApplicationRestrictions. This adds validation that the package name passed to setApplicationRestrictions is in the correct format. This will avoid an issue where a path could be entered resulting in a file being written to an unexpected place. Bug: 239701237 Test: atest UserManagerServiceTest Change-Id: I1ab2b7228470f10ec26fe3a608ae540cfc9e9a96 (cherry picked from commit 31a582490d6e8952d24f267df47d669e3861cf67) Merged-In: I1ab2b7228470f10ec26fe3a608ae540cfc9e9a96 (cherry picked from commit cfcfe6ca8c545f78603c05e23687f8638fd4b51d) (cherry picked from commit 91a821d2e4d80558cf39a6d728213d3df0826908) Merged-In: I1ab2b7228470f10ec26fe3a608ae540cfc9e9a96 --- .../android/server/pm/UserManagerService.java | 41 +++++++++++++++++++ .../server/pm/UserManagerServiceTest.java | 7 ++++ 2 files changed, 48 insertions(+) diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index a74569b55ae1c..f2037e066419a 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -88,6 +88,7 @@ import android.util.ArrayMap; import android.util.ArraySet; import android.util.AtomicFile; +import android.util.EventLog; import android.util.IndentingPrintWriter; import android.util.IntArray; import android.util.Slog; @@ -4500,6 +4501,13 @@ public Bundle getApplicationRestrictionsForUser(String packageName, @UserIdInt i public void setApplicationRestrictions(String packageName, Bundle restrictions, @UserIdInt int userId) { checkSystemOrRoot("set application restrictions"); + String validationResult = validateName(packageName); + if (validationResult != null) { + if (packageName.contains("../")) { + EventLog.writeEvent(0x534e4554, "239701237", -1, ""); + } + throw new IllegalArgumentException("Invalid package name: " + validationResult); + } if (restrictions != null) { restrictions.setDefusable(true); } @@ -4526,6 +4534,39 @@ public void setApplicationRestrictions(String packageName, Bundle restrictions, mContext.sendBroadcastAsUser(changeIntent, UserHandle.of(userId)); } + /** + * Check if the given name is valid. + * + * Note: the logic is taken from FrameworkParsingPackageUtils in master, edited to remove + * unnecessary parts. Copied here for a security fix. + * + * @param name The name to check. + * @return null if it's valid, error message if not + */ + @VisibleForTesting + static String validateName(String name) { + final int n = name.length(); + boolean front = true; + for (int i = 0; i < n; i++) { + final char c = name.charAt(i); + if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) { + front = false; + continue; + } + if (!front) { + if ((c >= '0' && c <= '9') || c == '_') { + continue; + } + if (c == '.') { + front = true; + continue; + } + } + return "bad character '" + c + "'"; + } + return null; + } + private int getUidForPackage(String packageName) { final long ident = Binder.clearCallingIdentity(); try { diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceTest.java index 6c1c019f504e3..658f168b608b5 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceTest.java @@ -86,6 +86,13 @@ public void testUserSystemPackageWhitelist() throws Exception { } } + public void testValidateName() { + assertNull(UserManagerService.validateName("android")); + assertNull(UserManagerService.validateName("com.company.myapp")); + assertNotNull(UserManagerService.validateName("/../../data")); + assertNotNull(UserManagerService.validateName("/dir")); + } + private Bundle createBundle() { Bundle result = new Bundle(); // Tests for 6 allowed types: Integer, Boolean, String, String[], Bundle and Parcelable[] From 068dce907568cf3031d634e7439eb51f2df0c450 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Mon, 7 Nov 2022 19:13:23 +0000 Subject: [PATCH 28/42] [DO NOT MERGE] Revert "Fix system zen rules by using owner package name if caller is system" This reverts commit 78245566a27882cce59c0d7cd4c60e1604392a3f. Reason for revert: broke DND schedules in multi-user mode b/257477671 Change-Id: I8a244a6ad0457ef2679b5216f48611d4025b9983 (cherry picked from commit 7deb6e4565789dafcdcafea7107d03662a5ee86c) Merged-In: I8a244a6ad0457ef2679b5216f48611d4025b9983 --- .../NotificationManagerService.java | 11 +----- .../NotificationManagerServiceTest.java | 37 ------------------- 2 files changed, 1 insertion(+), 47 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 0f50900e2bc0f..a0cb8ca49e784 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -4790,16 +4790,7 @@ public String addAutomaticZenRule(AutomaticZenRule automaticZenRule, String pkg) } enforcePolicyAccess(Binder.getCallingUid(), "addAutomaticZenRule"); - // If the caller is system, take the package name from the rule's owner rather than - // from the caller's package. - String rulePkg = pkg; - if (isCallingUidSystem()) { - if (automaticZenRule.getOwner() != null) { - rulePkg = automaticZenRule.getOwner().getPackageName(); - } - } - - return mZenModeHelper.addAutomaticZenRule(rulePkg, automaticZenRule, + return mZenModeHelper.addAutomaticZenRule(pkg, automaticZenRule, "addAutomaticZenRule"); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 3d6c6bab243b5..d52ae9b6a755b 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -6748,43 +6748,6 @@ public void testAutomaticZenRuleValidation_policyFilterAgreement() throws Except mBinderService.addAutomaticZenRule(rule, mContext.getPackageName()); } - @Test - public void testAddAutomaticZenRule_systemCallTakesPackageFromOwner() throws Exception { - mService.isSystemUid = true; - ZenModeHelper mockZenModeHelper = mock(ZenModeHelper.class); - when(mConditionProviders.isPackageOrComponentAllowed(anyString(), anyInt())) - .thenReturn(true); - mService.setZenHelper(mockZenModeHelper); - ComponentName owner = new ComponentName("android", "ProviderName"); - ZenPolicy zenPolicy = new ZenPolicy.Builder().allowAlarms(true).build(); - boolean isEnabled = true; - AutomaticZenRule rule = new AutomaticZenRule("test", owner, owner, mock(Uri.class), - zenPolicy, NotificationManager.INTERRUPTION_FILTER_PRIORITY, isEnabled); - mBinderService.addAutomaticZenRule(rule, "com.android.settings"); - - // verify that zen mode helper gets passed in a package name of "android" - verify(mockZenModeHelper).addAutomaticZenRule(eq("android"), eq(rule), anyString()); - } - - @Test - public void testAddAutomaticZenRule_nonSystemCallTakesPackageFromArg() throws Exception { - mService.isSystemUid = false; - ZenModeHelper mockZenModeHelper = mock(ZenModeHelper.class); - when(mConditionProviders.isPackageOrComponentAllowed(anyString(), anyInt())) - .thenReturn(true); - mService.setZenHelper(mockZenModeHelper); - ComponentName owner = new ComponentName("android", "ProviderName"); - ZenPolicy zenPolicy = new ZenPolicy.Builder().allowAlarms(true).build(); - boolean isEnabled = true; - AutomaticZenRule rule = new AutomaticZenRule("test", owner, owner, mock(Uri.class), - zenPolicy, NotificationManager.INTERRUPTION_FILTER_PRIORITY, isEnabled); - mBinderService.addAutomaticZenRule(rule, "another.package"); - - // verify that zen mode helper gets passed in the package name from the arg, not the owner - verify(mockZenModeHelper).addAutomaticZenRule( - eq("another.package"), eq(rule), anyString()); - } - @Test public void testAreNotificationsEnabledForPackage_crossUser() throws Exception { try { From cdefa691981cb566b95a2320e5efc67ef5525755 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Mon, 7 Nov 2022 19:09:41 +0000 Subject: [PATCH 29/42] [DO NOT MERGE] Revert "Check rule package name in ZenModeHelper.addAutomaticRule" This reverts commit 59732d6232d7d82d03897b25be0381c3c510db9b. Reason for revert: broke DND schedules in multi-user mode b/257477671 Change-Id: I33dd64f8686e60db332361b6cde2955c33cff8d0 (cherry picked from commit 3bba7fb1844c7d550b4b4a3665b641cffa1713fb) Merged-In: I33dd64f8686e60db332361b6cde2955c33cff8d0 --- .../server/notification/ZenModeHelper.java | 7 ++++- .../notification/ZenModeHelperTest.java | 30 ------------------- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index 9fd480134ae91..5e62496d5cca8 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -310,7 +310,7 @@ public AutomaticZenRule getAutomaticZenRule(String id) { public String addAutomaticZenRule(String pkg, AutomaticZenRule automaticZenRule, String reason) { - if (!ZenModeConfig.SYSTEM_AUTHORITY.equals(pkg)) { + if (!isSystemRule(automaticZenRule)) { PackageItemInfo component = getServiceInfo(automaticZenRule.getOwner()); if (component == null) { component = getActivityInfo(automaticZenRule.getConfigurationActivity()); @@ -566,6 +566,11 @@ protected void updateDefaultZenRules() { } } + private boolean isSystemRule(AutomaticZenRule rule) { + return rule.getOwner() != null + && ZenModeConfig.SYSTEM_AUTHORITY.equals(rule.getOwner().getPackageName()); + } + private ServiceInfo getServiceInfo(ComponentName owner) { Intent queryIntent = new Intent(); queryIntent.setComponent(owner); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java index 5c2730fb1075b..c0fb39c41676a 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java @@ -1667,36 +1667,6 @@ public void testAddAutomaticZenRule_beyondSystemLimit_differentComponents() { } } - @Test - public void testAddAutomaticZenRule_claimedSystemOwner() { - // Make sure anything that claims to have a "system" owner but not actually part of the - // system package still gets limited on number of rules - for (int i = 0; i < RULE_LIMIT_PER_PACKAGE; i++) { - ScheduleInfo si = new ScheduleInfo(); - si.startHour = i; - AutomaticZenRule zenRule = new AutomaticZenRule("name" + i, - new ComponentName("android", "ScheduleConditionProvider" + i), - null, // configuration activity - ZenModeConfig.toScheduleConditionId(si), - new ZenPolicy.Builder().build(), - NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); - String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); - assertNotNull(id); - } - try { - AutomaticZenRule zenRule = new AutomaticZenRule("name", - new ComponentName("android", "ScheduleConditionProviderFinal"), - null, // configuration activity - ZenModeConfig.toScheduleConditionId(new ScheduleInfo()), - new ZenPolicy.Builder().build(), - NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); - String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); - fail("allowed too many rules to be created"); - } catch (IllegalArgumentException e) { - // yay - } - } - @Test public void testAddAutomaticZenRule_CA() { AutomaticZenRule zenRule = new AutomaticZenRule("name", From 521d3e306b30c309570c6c4679df7502d81ade3f Mon Sep 17 00:00:00 2001 From: William Loh Date: Tue, 30 Aug 2022 00:11:15 +0000 Subject: [PATCH 30/42] Limit length and number of MIME types you can set Limit character length of MIME types to 255. If this length is exceeded then a IllegalArugmentException is thrown. The number of MIME types that can be set is also limited to 500 per MIME group with the number of total MIME Groups also limited to 500. A IllegalStateException is thrown if this number is exceeded. Bug: 237291548 Test: Installed and ran POC app from b/237291548 Change-Id: I1d57e674f778cfacdc89225ac3273c432a39af63 Merged-In: I1d57e674f778cfacdc89225ac3273c432a39af63 (cherry picked from commit 3ae3406b9706163073c282a8c4081faa32b606b2) Merged-In: I1d57e674f778cfacdc89225ac3273c432a39af63 --- .../android/content/pm/parsing/ParsingPackageImpl.java | 3 +++ .../core/java/com/android/server/pm/PackageSetting.java | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/core/java/android/content/pm/parsing/ParsingPackageImpl.java b/core/java/android/content/pm/parsing/ParsingPackageImpl.java index 5a7f21040d0a0..a6e1867fce0cc 100644 --- a/core/java/android/content/pm/parsing/ParsingPackageImpl.java +++ b/core/java/android/content/pm/parsing/ParsingPackageImpl.java @@ -1695,6 +1695,9 @@ private void addMimeGroupsFromComponent(ParsedComponent component) { for (int i = component.getIntents().size() - 1; i >= 0; i--) { IntentFilter filter = component.getIntents().get(i); for (int groupIndex = filter.countMimeGroups() - 1; groupIndex >= 0; groupIndex--) { + if (mimeGroups != null && mimeGroups.size() > 500) { + throw new IllegalStateException("Max limit on number of MIME Groups reached"); + } mimeGroups = ArrayUtils.add(mimeGroups, filter.getMimeGroup(groupIndex)); } } diff --git a/services/core/java/com/android/server/pm/PackageSetting.java b/services/core/java/com/android/server/pm/PackageSetting.java index 376326264d9c3..d8f902a9a0863 100644 --- a/services/core/java/com/android/server/pm/PackageSetting.java +++ b/services/core/java/com/android/server/pm/PackageSetting.java @@ -328,11 +328,20 @@ public boolean isMatch(int flags) { } public boolean setMimeGroup(String mimeGroup, List mimeTypes) { + for (String mimeType : mimeTypes) { + if (mimeType.length() > 255) { + throw new IllegalArgumentException("MIME type length exceeds 255 characters"); + } + } ArraySet oldMimeTypes = getMimeGroupInternal(mimeGroup); if (oldMimeTypes == null) { throw new IllegalArgumentException("Unknown MIME group " + mimeGroup + " for package " + name); } + if (mimeTypes.size() > 500) { + throw new IllegalStateException("Max limit on MIME types for MIME group " + + mimeGroup + " exceeded for package " + name); + } ArraySet newMimeTypes = new ArraySet<>(mimeTypes); boolean hasChanges = !newMimeTypes.equals(oldMimeTypes); From e9300a8b32252575cfddac2a45dd582574d1cbce Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Wed, 5 Oct 2022 16:28:20 -0700 Subject: [PATCH 31/42] Disable all A11yServices from an uninstalled package. Previous logic would exit the loop after removing the first service matching the uninstalled package. Bug: 243378132 Test: atest AccessibilityEndToEndTest Test: m sts; sts-tradefed run sts-dynamic-develop -m \ CtsAccessibilityServiceTestCases Change-Id: I4ba30345d8600674ee8a9ea3ff411aecbf3655a3 (cherry picked from commit e1f343acdeeddd9a08c9f6c832faf788ce101763) Merged-In: I4ba30345d8600674ee8a9ea3ff411aecbf3655a3 --- .../AccessibilityManagerService.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 42a2bda76c3c6..3659eedc401d1 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -506,25 +506,27 @@ public void onPackageRemoved(String packageName, int uid) { userState.mBindingServices.removeIf(filter); userState.mCrashedServices.removeIf(filter); final Iterator it = userState.mEnabledServices.iterator(); + boolean anyServiceRemoved = false; while (it.hasNext()) { final ComponentName comp = it.next(); final String compPkg = comp.getPackageName(); if (compPkg.equals(packageName)) { it.remove(); - // Update the enabled services setting. - persistComponentNamesToSettingLocked( - Settings.Secure.ENABLED_ACCESSIBILITY_SERVICES, - userState.mEnabledServices, userId); - // Update the touch exploration granted services setting. userState.mTouchExplorationGrantedServices.remove(comp); - persistComponentNamesToSettingLocked( - Settings.Secure. - TOUCH_EXPLORATION_GRANTED_ACCESSIBILITY_SERVICES, - userState.mTouchExplorationGrantedServices, userId); - onUserStateChangedLocked(userState); - return; + anyServiceRemoved = true; } } + if (anyServiceRemoved) { + // Update the enabled services setting. + persistComponentNamesToSettingLocked( + Settings.Secure.ENABLED_ACCESSIBILITY_SERVICES, + userState.mEnabledServices, userId); + // Update the touch exploration granted services setting. + persistComponentNamesToSettingLocked( + Settings.Secure.TOUCH_EXPLORATION_GRANTED_ACCESSIBILITY_SERVICES, + userState.mTouchExplorationGrantedServices, userId); + onUserStateChangedLocked(userState); + } } } From 847c62675333e85156503fb8fdfa24351e178447 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Wed, 12 Oct 2022 14:27:46 +0000 Subject: [PATCH 32/42] [DO NOT MERGE] Fix conditionId string trimming in AutomaticZenRule This change only applies to S branches and earlier. Bug: 253085433 Bug: 242703460 Bug: 242703505 Bug: 242703780 Bug: 242704043 Bug: 243794204 Test: AutomaticZenRuleTest Change-Id: Iae423d93b777df8946ecf1c3baf640fcf74990ec Merged-In: Iae423d93b777df8946ecf1c3baf640fcf74990ec (cherry picked from commit 7533d0420d85d56ec42bdb30a2ef1ae55ae95080) Merged-In: Iae423d93b777df8946ecf1c3baf640fcf74990ec --- core/java/android/app/AutomaticZenRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/android/app/AutomaticZenRule.java b/core/java/android/app/AutomaticZenRule.java index 27a9a27e59a26..2573d98576716 100644 --- a/core/java/android/app/AutomaticZenRule.java +++ b/core/java/android/app/AutomaticZenRule.java @@ -125,7 +125,7 @@ public AutomaticZenRule(Parcel source) { name = getTrimmedString(source.readString()); } interruptionFilter = source.readInt(); - conditionId = source.readParcelable(null); + conditionId = getTrimmedUri(source.readParcelable(null)); owner = getTrimmedComponentName(source.readParcelable(null)); configurationActivity = getTrimmedComponentName(source.readParcelable(null)); creationTime = source.readLong(); From 6f692e2cd97d22ae1cd4a790c8ed56e836e1ffec Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Wed, 17 Aug 2022 09:37:18 -0700 Subject: [PATCH 33/42] [RESTRICT AUTOMERGE] [SettingsProvider] mem limit should be checked before settings are updated Previously, a setting is updated before the memory usage limit check, which can be exploited by malicious apps and cause OoM DoS. This CL changes the logic to checkMemLimit -> update -> updateMemUsage. BUG: 239415861 Test: atest com.android.providers.settings.SettingsStateTest (cherry picked from commit 8eeb92950f4a7012d4cf282106a1418fd211f475) Merged-In: I20551a2dba9aa79efa0c064824f349f551c2c2e4 Change-Id: I20551a2dba9aa79efa0c064824f349f551c2c2e4 (cherry picked from commit 966b597383d1f5cbbc943affeb24b9b225b2ddae) Merged-In: I20551a2dba9aa79efa0c064824f349f551c2c2e4 --- .../providers/settings/SettingsState.java | 75 ++++++++++++------- .../providers/settings/SettingsStateTest.java | 38 ++++++++++ 2 files changed, 86 insertions(+), 27 deletions(-) diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index f04acd09ae140..227fa9d0bdc4f 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -375,9 +375,11 @@ public void resetSettingDefaultValueLocked(String name) { Setting newSetting = new Setting(name, oldSetting.getValue(), null, oldSetting.getPackageName(), oldSetting.getTag(), false, oldSetting.getId()); - mSettings.put(name, newSetting); - updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue, + int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue, newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue()); + checkNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize); + mSettings.put(name, newSetting); + updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize); scheduleWriteIfNeededLocked(); } } @@ -410,6 +412,12 @@ public boolean insertSettingLocked(String name, String value, String tag, Setting oldState = mSettings.get(name); String oldValue = (oldState != null) ? oldState.value : null; String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null; + String newDefaultValue = makeDefault ? value : oldDefaultValue; + + int newSize = getNewMemoryUsagePerPackageLocked(packageName, oldValue, value, + oldDefaultValue, newDefaultValue); + checkNewMemoryUsagePerPackageLocked(packageName, newSize); + Setting newState; if (oldState != null) { @@ -430,8 +438,7 @@ oldValue, tag, makeDefault, getUserIdFromKey(mKey), addHistoricalOperationLocked(HISTORICAL_OPERATION_UPDATE, newState); - updateMemoryUsagePerPackageLocked(packageName, oldValue, value, - oldDefaultValue, newState.getDefaultValue()); + updateMemoryUsagePerPackageLocked(packageName, newSize); scheduleWriteIfNeededLocked(); @@ -552,13 +559,14 @@ public boolean deleteSettingLocked(String name) { } Setting oldState = mSettings.remove(name); + int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value, + null, oldState.defaultValue, null); FrameworkStatsLog.write(FrameworkStatsLog.SETTING_CHANGED, name, /* value= */ "", /* newValue= */ "", oldState.value, /* tag */ "", false, getUserIdFromKey(mKey), FrameworkStatsLog.SETTING_CHANGED__REASON__DELETED); - updateMemoryUsagePerPackageLocked(oldState.packageName, oldState.value, - null, oldState.defaultValue, null); + updateMemoryUsagePerPackageLocked(oldState.packageName, newSize); addHistoricalOperationLocked(HISTORICAL_OPERATION_DELETE, oldState); @@ -579,16 +587,18 @@ public boolean resetSettingLocked(String name) { Setting oldSetting = new Setting(setting); String oldValue = setting.getValue(); String oldDefaultValue = setting.getDefaultValue(); + String newValue = oldDefaultValue; + String newDefaultValue = oldDefaultValue; + + int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, oldValue, + newValue, oldDefaultValue, newDefaultValue); + checkNewMemoryUsagePerPackageLocked(setting.packageName, newSize); if (!setting.reset()) { return false; } - String newValue = setting.getValue(); - String newDefaultValue = setting.getDefaultValue(); - - updateMemoryUsagePerPackageLocked(setting.packageName, oldValue, - newValue, oldDefaultValue, newDefaultValue); + updateMemoryUsagePerPackageLocked(setting.packageName, newSize); addHistoricalOperationLocked(HISTORICAL_OPERATION_RESET, oldSetting); @@ -696,38 +706,49 @@ public void dumpHistoricalOperations(PrintWriter pw) { } @GuardedBy("mLock") - private void updateMemoryUsagePerPackageLocked(String packageName, String oldValue, - String newValue, String oldDefaultValue, String newDefaultValue) { - if (mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED) { - return; - } + private boolean isExemptFromMemoryUsageCap(String packageName) { + return mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED + || SYSTEM_PACKAGE_NAME.equals(packageName); + } - if (SYSTEM_PACKAGE_NAME.equals(packageName)) { + @GuardedBy("mLock") + private void checkNewMemoryUsagePerPackageLocked(String packageName, int newSize) + throws IllegalStateException { + if (isExemptFromMemoryUsageCap(packageName)) { return; } + if (newSize > mMaxBytesPerAppPackage) { + throw new IllegalStateException("You are adding too many system settings. " + + "You should stop using system settings for app specific data" + + " package: " + packageName); + } + } + @GuardedBy("mLock") + private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValue, + String newValue, String oldDefaultValue, String newDefaultValue) { + if (isExemptFromMemoryUsageCap(packageName)) { + return 0; + } + final Integer currentSize = mPackageToMemoryUsage.get(packageName); final int oldValueSize = (oldValue != null) ? oldValue.length() : 0; final int newValueSize = (newValue != null) ? newValue.length() : 0; final int oldDefaultValueSize = (oldDefaultValue != null) ? oldDefaultValue.length() : 0; final int newDefaultValueSize = (newDefaultValue != null) ? newDefaultValue.length() : 0; final int deltaSize = newValueSize + newDefaultValueSize - oldValueSize - oldDefaultValueSize; + return Math.max((currentSize != null) ? currentSize + deltaSize : deltaSize, 0); + } - Integer currentSize = mPackageToMemoryUsage.get(packageName); - final int newSize = Math.max((currentSize != null) - ? currentSize + deltaSize : deltaSize, 0); - - if (newSize > mMaxBytesPerAppPackage) { - throw new IllegalStateException("You are adding too many system settings. " - + "You should stop using system settings for app specific data" - + " package: " + packageName); + @GuardedBy("mLock") + private void updateMemoryUsagePerPackageLocked(String packageName, int newSize) { + if (isExemptFromMemoryUsageCap(packageName)) { + return; } - if (DEBUG) { Slog.i(LOG_TAG, "Settings for package: " + packageName + " size: " + newSize + " bytes."); } - mPackageToMemoryUsage.put(packageName, newSize); } diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java index 69eb7133f46fe..66b809aeae308 100644 --- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java +++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java @@ -20,6 +20,8 @@ import android.util.TypedXmlSerializer; import android.util.Xml; +import com.google.common.base.Strings; + import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; @@ -276,4 +278,40 @@ private SettingsState getSettingStateObject() { settingsState.setVersionLocked(SettingsState.SETTINGS_VERSION_NEW_ENCODING); return settingsState; } + + public void testInsertSetting_memoryUsage() { + SettingsState settingsState = new SettingsState(getContext(), mLock, mSettingsFile, 1, + SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper()); + // No exception should be thrown when there is no cap + settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 20001), + null, false, "p1"); + settingsState.deleteSettingLocked(SETTING_NAME); + + settingsState = new SettingsState(getContext(), mLock, mSettingsFile, 1, + SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper()); + // System package doesn't have memory usage limit + settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 20001), + null, false, SYSTEM_PACKAGE); + settingsState.deleteSettingLocked(SETTING_NAME); + + // Should not throw if usage is under the cap + settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 19999), + null, false, "p1"); + settingsState.deleteSettingLocked(SETTING_NAME); + try { + settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 20001), + null, false, "p1"); + fail("Should throw because it exceeded per package memory usage"); + } catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("p1")); + } + try { + settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 20001), + null, false, "p1"); + fail("Should throw because it exceeded per package memory usage"); + } catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("p1")); + } + assertTrue(settingsState.getSettingLocked(SETTING_NAME).isNull()); + } } From da9ce922b9eb1ac5139a1baf957cebb67be46015 Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Tue, 8 Feb 2022 06:22:47 +0000 Subject: [PATCH 34/42] Backport missing permission check for querying main activity intent - This was fixed in T in ag/16820166, but the original code was submitted in S. This ensures that the caller of this method is either holding the ACCESS_SHORTCUTS permission or is the default launcher. Bug: 229256049 Test: atest WMShellUnitTests Change-Id: Ib233ad754a6c6e3c4e0d0e10ed788ab8e055cccc Merged-In: Ib233ad754a6c6e3c4e0d0e10ed788ab8e055cccc (cherry picked from commit f4ed441e180d7113b5f6ebfe711e61a2dd3fd8b1) (cherry picked from commit b3192809643eff948d9457c8a7b36b968a7388a1) Merged-In: Ib233ad754a6c6e3c4e0d0e10ed788ab8e055cccc --- core/java/android/content/pm/ILauncherApps.aidl | 4 ++-- core/java/android/content/pm/LauncherApps.java | 3 ++- .../core/java/com/android/server/pm/LauncherAppsService.java | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/core/java/android/content/pm/ILauncherApps.aidl b/core/java/android/content/pm/ILauncherApps.aidl index 37fd3ffdeafa5..2eaadb05276c9 100644 --- a/core/java/android/content/pm/ILauncherApps.aidl +++ b/core/java/android/content/pm/ILauncherApps.aidl @@ -56,8 +56,8 @@ interface ILauncherApps { void startActivityAsUser(in IApplicationThread caller, String callingPackage, String callingFeatureId, in ComponentName component, in Rect sourceBounds, in Bundle opts, in UserHandle user); - PendingIntent getActivityLaunchIntent(in ComponentName component, in Bundle opts, - in UserHandle user); + PendingIntent getActivityLaunchIntent(String callingPackage, in ComponentName component, + in Bundle opts, in UserHandle user); void showAppDetailsAsUser(in IApplicationThread caller, String callingPackage, String callingFeatureId, in ComponentName component, in Rect sourceBounds, in Bundle opts, in UserHandle user); diff --git a/core/java/android/content/pm/LauncherApps.java b/core/java/android/content/pm/LauncherApps.java index 0f9acadb11f98..4b6c586266a97 100644 --- a/core/java/android/content/pm/LauncherApps.java +++ b/core/java/android/content/pm/LauncherApps.java @@ -752,7 +752,8 @@ public PendingIntent getMainActivityLaunchIntent(@NonNull ComponentName componen } try { // due to b/209607104, startActivityOptions will be ignored - return mService.getActivityLaunchIntent(component, null /* opts */, user); + return mService.getActivityLaunchIntent(mContext.getPackageName(), component, + null /* opts */, user); } catch (RemoteException re) { throw re.rethrowFromSystemServer(); } diff --git a/services/core/java/com/android/server/pm/LauncherAppsService.java b/services/core/java/com/android/server/pm/LauncherAppsService.java index 9253837ed971e..3faffad2e8542 100644 --- a/services/core/java/com/android/server/pm/LauncherAppsService.java +++ b/services/core/java/com/android/server/pm/LauncherAppsService.java @@ -1119,8 +1119,9 @@ public void startSessionDetailsActivityAsUser(IApplicationThread caller, } @Override - public PendingIntent getActivityLaunchIntent(ComponentName component, Bundle opts, - UserHandle user) { + public PendingIntent getActivityLaunchIntent(String callingPackage, ComponentName component, + Bundle opts, UserHandle user) { + ensureShortcutPermission(callingPackage); if (!canAccessProfile(user.getIdentifier(), "Cannot start activity")) { throw new ActivityNotFoundException("Activity could not be found"); } From 2dc4541d438d65f86ec24b5f8709a147936d9aa2 Mon Sep 17 00:00:00 2001 From: Nate Myren Date: Thu, 22 Sep 2022 15:23:24 -0700 Subject: [PATCH 35/42] RESTRICT AUTOMERGE Validate permission tree size on permission update Bug: 242537498 Test: manual Change-Id: I15343e84c1802d6b89249106263319a6539fa73b (cherry picked from commit 1d86c8b29922525972559e00e26c1fcd6f496353) Merged-In: I15343e84c1802d6b89249106263319a6539fa73b --- .../android/server/pm/permission/PermissionManagerService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index d307d06ef106f..de79da2ae4b11 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -657,8 +657,8 @@ public boolean addPermission(PermissionInfo info, boolean async) { Permission bp = mRegistry.getPermission(info.name); added = bp == null; int fixedLevel = PermissionInfo.fixProtectionLevel(info.protectionLevel); + enforcePermissionCapLocked(info, tree); if (added) { - enforcePermissionCapLocked(info, tree); bp = new Permission(info.name, tree.getPackageName(), Permission.TYPE_DYNAMIC); } else if (!bp.isDynamic()) { throw new SecurityException("Not allowed to modify non-dynamic permission " From fd3e1a61a9f8f023da65e80f8a43009944a75793 Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Tue, 11 Oct 2022 18:08:11 -0700 Subject: [PATCH 36/42] [RESTRICT AUTOMERGE][SettingsProvider] key size limit for mutating settings Prior to targetSdk 22, apps could add random system settings keys which opens an opportunity for OOM attacks. This CL adds a key size limit. BUG: 239415997 Test: manual; will add cts test Merged-In: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc Change-Id: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc (cherry picked from commit 783bcba343c480f6ccedaaff41ba7171a1082e0c) (cherry picked from commit 0123e873d2e5f4c88aaa6bdf2e287461903fa6e5) Merged-In: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc --- .../providers/settings/SettingsState.java | 40 +++++--- .../providers/settings/SettingsStateTest.java | 94 ++++++++++++++++++- 2 files changed, 120 insertions(+), 14 deletions(-) diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index 227fa9d0bdc4f..e2beef0139bfb 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -47,6 +47,7 @@ import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.FrameworkStatsLog; import libcore.io.IoUtils; @@ -375,8 +376,8 @@ public void resetSettingDefaultValueLocked(String name) { Setting newSetting = new Setting(name, oldSetting.getValue(), null, oldSetting.getPackageName(), oldSetting.getTag(), false, oldSetting.getId()); - int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue, - newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue()); + int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), 0, + oldValue, newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue()); checkNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize); mSettings.put(name, newSetting); updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize); @@ -414,8 +415,9 @@ public boolean insertSettingLocked(String name, String value, String tag, String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null; String newDefaultValue = makeDefault ? value : oldDefaultValue; - int newSize = getNewMemoryUsagePerPackageLocked(packageName, oldValue, value, - oldDefaultValue, newDefaultValue); + int newSize = getNewMemoryUsagePerPackageLocked(packageName, + oldValue == null ? name.length() : 0 /* deltaKeySize */, + oldValue, value, oldDefaultValue, newDefaultValue); checkNewMemoryUsagePerPackageLocked(packageName, newSize); Setting newState; @@ -559,8 +561,12 @@ public boolean deleteSettingLocked(String name) { } Setting oldState = mSettings.remove(name); - int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value, - null, oldState.defaultValue, null); + if (oldState == null) { + return false; + } + int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, + -name.length() /* deltaKeySize */, + oldState.value, null, oldState.defaultValue, null); FrameworkStatsLog.write(FrameworkStatsLog.SETTING_CHANGED, name, /* value= */ "", /* newValue= */ "", oldState.value, /* tag */ "", false, getUserIdFromKey(mKey), @@ -583,15 +589,16 @@ public boolean resetSettingLocked(String name) { } Setting setting = mSettings.get(name); + if (setting == null) { + return false; + } Setting oldSetting = new Setting(setting); String oldValue = setting.getValue(); String oldDefaultValue = setting.getDefaultValue(); - String newValue = oldDefaultValue; - String newDefaultValue = oldDefaultValue; - int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, oldValue, - newValue, oldDefaultValue, newDefaultValue); + int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, 0, oldValue, + oldDefaultValue, oldDefaultValue, oldDefaultValue); checkNewMemoryUsagePerPackageLocked(setting.packageName, newSize); if (!setting.reset()) { @@ -725,8 +732,8 @@ private void checkNewMemoryUsagePerPackageLocked(String packageName, int newSize } @GuardedBy("mLock") - private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValue, - String newValue, String oldDefaultValue, String newDefaultValue) { + private int getNewMemoryUsagePerPackageLocked(String packageName, int deltaKeySize, + String oldValue, String newValue, String oldDefaultValue, String newDefaultValue) { if (isExemptFromMemoryUsageCap(packageName)) { return 0; } @@ -735,7 +742,7 @@ private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValu final int newValueSize = (newValue != null) ? newValue.length() : 0; final int oldDefaultValueSize = (oldDefaultValue != null) ? oldDefaultValue.length() : 0; final int newDefaultValueSize = (newDefaultValue != null) ? newDefaultValue.length() : 0; - final int deltaSize = newValueSize + newDefaultValueSize + final int deltaSize = deltaKeySize + newValueSize + newDefaultValueSize - oldValueSize - oldDefaultValueSize; return Math.max((currentSize != null) ? currentSize + deltaSize : deltaSize, 0); } @@ -1556,4 +1563,11 @@ private static boolean isSystemPackage(@Nullable ApplicationInfo aInfo) { } return false; } + + @VisibleForTesting + public int getMemoryUsage(String packageName) { + synchronized (mLock) { + return mPackageToMemoryUsage.getOrDefault(packageName, 0); + } + } } diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java index 66b809aeae308..f6d43292eafe1 100644 --- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java +++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java @@ -295,7 +295,7 @@ public void testInsertSetting_memoryUsage() { settingsState.deleteSettingLocked(SETTING_NAME); // Should not throw if usage is under the cap - settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 19999), + settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 19975), null, false, "p1"); settingsState.deleteSettingLocked(SETTING_NAME); try { @@ -313,5 +313,97 @@ public void testInsertSetting_memoryUsage() { assertTrue(ex.getMessage().contains("p1")); } assertTrue(settingsState.getSettingLocked(SETTING_NAME).isNull()); + try { + settingsState.insertSettingLocked(Strings.repeat("A", 20001), "", + null, false, "p1"); + fail("Should throw because it exceeded per package memory usage"); + } catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("You are adding too many system settings")); + } + } + + public void testMemoryUsagePerPackage() { + SettingsState settingsState = new SettingsState(getContext(), mLock, mSettingsFile, 1, + SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper()); + + // Test inserting one key with default + final String testKey1 = SETTING_NAME; + final String testValue1 = Strings.repeat("A", 100); + settingsState.insertSettingLocked(testKey1, testValue1, null, true, TEST_PACKAGE); + int expectedMemUsage = testKey1.length() + testValue1.length() + + testValue1.length() /* size for default */; + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test inserting another key + final String testKey2 = SETTING_NAME + "2"; + settingsState.insertSettingLocked(testKey2, testValue1, null, false, TEST_PACKAGE); + expectedMemUsage += testKey2.length() + testValue1.length(); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test updating first key with new default + final String testValue2 = Strings.repeat("A", 300); + settingsState.insertSettingLocked(testKey1, testValue2, null, true, TEST_PACKAGE); + expectedMemUsage += (testValue2.length() - testValue1.length()) * 2; + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test updating first key without new default + final String testValue3 = Strings.repeat("A", 50); + settingsState.insertSettingLocked(testKey1, testValue3, null, false, TEST_PACKAGE); + expectedMemUsage -= testValue2.length() - testValue3.length(); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test updating second key + settingsState.insertSettingLocked(testKey2, testValue2, null, false, TEST_PACKAGE); + expectedMemUsage -= testValue1.length() - testValue2.length(); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test resetting key + settingsState.resetSettingLocked(testKey1); + expectedMemUsage += testValue2.length() - testValue3.length(); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test resetting default value + settingsState.resetSettingDefaultValueLocked(testKey1); + expectedMemUsage -= testValue2.length(); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test deletion + settingsState.deleteSettingLocked(testKey2); + expectedMemUsage -= testValue2.length() + testKey2.length() /* key is deleted too */; + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test another package with a different key + final String testPackage2 = TEST_PACKAGE + "2"; + final String testKey3 = SETTING_NAME + "3"; + settingsState.insertSettingLocked(testKey3, testValue1, null, true, testPackage2); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + final int expectedMemUsage2 = testKey3.length() + testValue1.length() * 2; + assertEquals(expectedMemUsage2, settingsState.getMemoryUsage(testPackage2)); + + // Test system package + settingsState.insertSettingLocked(testKey1, testValue1, null, true, SYSTEM_PACKAGE); + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + assertEquals(expectedMemUsage2, settingsState.getMemoryUsage(testPackage2)); + assertEquals(0, settingsState.getMemoryUsage(SYSTEM_PACKAGE)); + + // Test invalid value + try { + settingsState.insertSettingLocked(testKey1, Strings.repeat("A", 20001), null, false, + TEST_PACKAGE); + fail("Should throw because it exceeded per package memory usage"); + } catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("You are adding too many system settings")); + } + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); + + // Test invalid key + try { + settingsState.insertSettingLocked(Strings.repeat("A", 20001), "", null, false, + TEST_PACKAGE); + fail("Should throw because it exceeded per package memory usage"); + } catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("You are adding too many system settings")); + } + assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); } } From 32186174a99700f4d097ae9dd6609cf9defc5252 Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Wed, 28 Sep 2022 06:46:29 +0000 Subject: [PATCH 37/42] [RESTRICT AUTOMERGE] Trim the activity info of another uid if no privilege The activity info could be from another uid which is different from the app that hosts the task. The information should be trimmed if the caller app doesn't have the privilege. Bug: 243130512 Test: verified locally Test: atest RecentTasksTest Change-Id: Ia343ac70e5bb9aeae718fca6674e1ca491a14512 Merged-In: Ia343ac70e5bb9aeae718fca6674e1ca491a14512 (cherry picked from commit fa8d6362348738284b3f33a13e1fa5cdd0af67b2) Merged-In: Ia343ac70e5bb9aeae718fca6674e1ca491a14512 --- .../com/android/server/wm/AppTaskImpl.java | 2 +- .../com/android/server/wm/RecentTasks.java | 8 ++++-- .../com/android/server/wm/RunningTasks.java | 4 +++ .../core/java/com/android/server/wm/Task.java | 21 +++++++++++++++ .../android/server/wm/RecentTasksTest.java | 27 +++++++++++++++---- 5 files changed, 54 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/wm/AppTaskImpl.java b/services/core/java/com/android/server/wm/AppTaskImpl.java index 5589396119054..3c9adfb9c4bd5 100644 --- a/services/core/java/com/android/server/wm/AppTaskImpl.java +++ b/services/core/java/com/android/server/wm/AppTaskImpl.java @@ -84,7 +84,7 @@ public ActivityManager.RecentTaskInfo getTaskInfo() { throw new IllegalArgumentException("Unable to find task ID " + mTaskId); } return mService.getRecentTasks().createRecentTaskInfo(task, - false /* stripExtras */); + false /* stripExtras */, true /* getTasksAllowed */); } finally { Binder.restoreCallingIdentity(origId); } diff --git a/services/core/java/com/android/server/wm/RecentTasks.java b/services/core/java/com/android/server/wm/RecentTasks.java index dca0bbda78cfc..1a3d621794d38 100644 --- a/services/core/java/com/android/server/wm/RecentTasks.java +++ b/services/core/java/com/android/server/wm/RecentTasks.java @@ -974,7 +974,7 @@ private ArrayList getRecentTasksImpl(int maxNum, continue; } - res.add(createRecentTaskInfo(task, true /* stripExtras */)); + res.add(createRecentTaskInfo(task, true /* stripExtras */, getTasksAllowed)); } return res; } @@ -1890,7 +1890,8 @@ void dump(PrintWriter pw, boolean dumpAll, String dumpPackage) { /** * Creates a new RecentTaskInfo from a Task. */ - ActivityManager.RecentTaskInfo createRecentTaskInfo(Task tr, boolean stripExtras) { + ActivityManager.RecentTaskInfo createRecentTaskInfo(Task tr, boolean stripExtras, + boolean getTasksAllowed) { final ActivityManager.RecentTaskInfo rti = new ActivityManager.RecentTaskInfo(); // If the recent Task is detached, we consider it will be re-attached to the default // TaskDisplayArea because we currently only support recent overview in the default TDA. @@ -1902,6 +1903,9 @@ ActivityManager.RecentTaskInfo createRecentTaskInfo(Task tr, boolean stripExtras rti.id = rti.isRunning ? rti.taskId : INVALID_TASK_ID; rti.persistentId = rti.taskId; rti.lastSnapshotData.set(tr.mLastTaskSnapshotData); + if (!getTasksAllowed) { + Task.trimIneffectiveInfo(tr, rti); + } // Fill in organized child task info for the task created by organizer. if (tr.mCreatedByOrganizer) { diff --git a/services/core/java/com/android/server/wm/RunningTasks.java b/services/core/java/com/android/server/wm/RunningTasks.java index 9864297de5294..7acd5152c2919 100644 --- a/services/core/java/com/android/server/wm/RunningTasks.java +++ b/services/core/java/com/android/server/wm/RunningTasks.java @@ -150,6 +150,10 @@ private RunningTaskInfo createRunningTaskInfo(Task task) { task.fillTaskInfo(rti, !mKeepIntentExtra); // Fill in some deprecated values rti.id = rti.taskId; + + if (!mAllowed) { + Task.trimIneffectiveInfo(task, rti); + } return rti; } } diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index 2af7817cacaa0..d96c04fcc2671 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -3484,6 +3484,27 @@ void fillTaskInfo(TaskInfo info, boolean stripExtras, @Nullable TaskDisplayArea info.mTopActivityLocusId = topRecord != null ? topRecord.getLocusId() : null; } + /** + * Removes the activity info if the activity belongs to a different uid, which is + * different from the app that hosts the task. + */ + static void trimIneffectiveInfo(Task task, TaskInfo info) { + final ActivityRecord baseActivity = task.getActivity(r -> !r.finishing, + false /* traverseTopToBottom */); + final int baseActivityUid = + baseActivity != null ? baseActivity.getUid() : task.effectiveUid; + + if (info.topActivityInfo != null + && task.effectiveUid != info.topActivityInfo.applicationInfo.uid) { + info.topActivity = null; + info.topActivityInfo = null; + } + + if (task.effectiveUid != baseActivityUid) { + info.baseActivity = null; + } + } + @Nullable PictureInPictureParams getPictureInPictureParams() { return getPictureInPictureParams(getTopMostTask()); } diff --git a/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java b/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java index 284728397c9fe..f5e16a903a966 100644 --- a/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java @@ -30,6 +30,7 @@ import static android.content.pm.ActivityInfo.LAUNCH_MULTIPLE; import static android.content.pm.ActivityInfo.LAUNCH_SINGLE_INSTANCE; import static android.content.res.Configuration.ORIENTATION_PORTRAIT; +import static android.os.Process.NOBODY_UID; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; @@ -1195,21 +1196,35 @@ public void testLastSnapshotData_notSet() { @Test public void testCreateRecentTaskInfo_detachedTask() { - final Task task = createTaskBuilder(".Task").setCreateActivity(true).build(); + final Task task = createTaskBuilder(".Task").build(); + new ActivityBuilder(mSupervisor.mService) + .setTask(task) + .setUid(NOBODY_UID) + .setComponent(new ComponentName("com.foo", ".BarActivity")) + .build(); final TaskDisplayArea tda = task.getDisplayArea(); assertTrue(task.isAttached()); assertTrue(task.supportsMultiWindow()); - RecentTaskInfo info = mRecentTasks.createRecentTaskInfo(task, true); + RecentTaskInfo info = mRecentTasks.createRecentTaskInfo(task, true /* stripExtras */, + true /* getTasksAllowed */); assertTrue(info.supportsMultiWindow); assertTrue(info.supportsSplitScreenMultiWindow); + info = mRecentTasks.createRecentTaskInfo(task, true /* stripExtras */, + false /* getTasksAllowed */); + + assertTrue(info.topActivity == null); + assertTrue(info.topActivityInfo == null); + assertTrue(info.baseActivity == null); + // The task can be put in split screen even if it is not attached now. task.removeImmediately(); - info = mRecentTasks.createRecentTaskInfo(task, true); + info = mRecentTasks.createRecentTaskInfo(task, true /* stripExtras */, + true /* getTasksAllowed */); assertTrue(info.supportsMultiWindow); assertTrue(info.supportsSplitScreenMultiWindow); @@ -1219,7 +1234,8 @@ public void testCreateRecentTaskInfo_detachedTask() { doReturn(false).when(tda).supportsNonResizableMultiWindow(); doReturn(false).when(task).isResizeable(); - info = mRecentTasks.createRecentTaskInfo(task, true); + info = mRecentTasks.createRecentTaskInfo(task, true /* stripExtras */, + true /* getTasksAllowed */); assertFalse(info.supportsMultiWindow); assertFalse(info.supportsSplitScreenMultiWindow); @@ -1228,7 +1244,8 @@ public void testCreateRecentTaskInfo_detachedTask() { // the device supports it. doReturn(true).when(tda).supportsNonResizableMultiWindow(); - info = mRecentTasks.createRecentTaskInfo(task, true); + info = mRecentTasks.createRecentTaskInfo(task, true /* stripExtras */, + true /* getTasksAllowed */); assertTrue(info.supportsMultiWindow); assertTrue(info.supportsSplitScreenMultiWindow); From 097ee0bc92fb19f99877390735442b264ba9874a Mon Sep 17 00:00:00 2001 From: Nate Myren Date: Fri, 23 Sep 2022 12:04:57 -0700 Subject: [PATCH 38/42] RESTRICT AUTOMERGE Revoke SYSTEM_ALERT_WINDOW on upgrade past api 23 Bug: 221040577 Test: atest PermissionTest23#testPre23AppsWithSystemAlertWindowGetDeniedOnUpgrade Change-Id: I4b4605aaae107875811070dea6d031c5d9f25c96 (cherry picked from commit 5e80fcf8c423f288a87d727f48ae38112177d716) Merged-In: I4b4605aaae107875811070dea6d031c5d9f25c96 --- .../permission/PermissionManagerService.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index de79da2ae4b11..211657a32112f 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -2254,6 +2254,46 @@ private void revokeStoragePermissionsIfScopeExpandedInternal( } + /** + * If the package was below api 23, got the SYSTEM_ALERT_WINDOW permission automatically, and + * then updated past api 23, and the app does not satisfy any of the other SAW permission flags, + * the permission should be revoked. + * + * @param newPackage The new package that was installed + * @param oldPackage The old package that was updated + */ + private void revokeSystemAlertWindowIfUpgradedPast23( + @NonNull AndroidPackage newPackage, + @NonNull AndroidPackage oldPackage) { + if (oldPackage.getTargetSdkVersion() >= Build.VERSION_CODES.M + || newPackage.getTargetSdkVersion() < Build.VERSION_CODES.M + || !newPackage.getRequestedPermissions() + .contains(Manifest.permission.SYSTEM_ALERT_WINDOW)) { + return; + } + + Permission saw; + synchronized (mLock) { + saw = mRegistry.getPermission(Manifest.permission.SYSTEM_ALERT_WINDOW); + } + final PackageSetting ps = (PackageSetting) + mPackageManagerInt.getPackageSetting(newPackage.getPackageName()); + if (shouldGrantPermissionByProtectionFlags(newPackage, ps, saw, new ArraySet<>()) + || shouldGrantPermissionBySignature(newPackage, saw)) { + return; + } + for (int userId : getAllUserIds()) { + try { + revokePermissionFromPackageForUser(newPackage.getPackageName(), + Manifest.permission.SYSTEM_ALERT_WINDOW, false, userId, + mDefaultPermissionCallback); + } catch (IllegalStateException | SecurityException e) { + Log.e(TAG, "unable to revoke SYSTEM_ALERT_WINDOW for " + + newPackage.getPackageName() + " user " + userId, e); + } + } + } + /** * We might auto-grant permissions if any permission of the group is already granted. Hence if * the group of a granted permission changes we need to revoke it to avoid having permissions of @@ -4831,6 +4871,7 @@ private void onPackageAddedInternal(@NonNull AndroidPackage pkg, boolean isInsta if (hasOldPkg) { revokeRuntimePermissionsIfGroupChangedInternal(pkg, oldPkg); revokeStoragePermissionsIfScopeExpandedInternal(pkg, oldPkg); + revokeSystemAlertWindowIfUpgradedPast23(pkg, oldPkg); } if (hasPermissionDefinitionChanges) { revokeRuntimePermissionsIfPermissionDefinitionChangedInternal( From ebc061fa7e77689a7b7759225f6b62b9b1786d0e Mon Sep 17 00:00:00 2001 From: Khoa Hong Date: Fri, 23 Sep 2022 17:05:39 +0800 Subject: [PATCH 39/42] Add protections against queueing a UsbRequest when the underlying UsbDeviceConnection is closed. Bug: 204584366 Test: CTS Verifier: USB Accessory Test & USB Device Test Test: No HWASan use-after-free reports with a test app Change-Id: Ia3a9b10349efc0236b1539c81465f479cb32e02b (cherry picked from commit 1691b54b1fda4239249a3871d2c17ed1ec753061) Merged-In: Ia3a9b10349efc0236b1539c81465f479cb32e02b --- .../hardware/usb/UsbDeviceConnection.java | 28 ++++++++ .../java/android/hardware/usb/UsbRequest.java | 68 ++++++++++++++++--- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/core/java/android/hardware/usb/UsbDeviceConnection.java b/core/java/android/hardware/usb/UsbDeviceConnection.java index 1c35cb66ada8f..5df8512ee0e7c 100644 --- a/core/java/android/hardware/usb/UsbDeviceConnection.java +++ b/core/java/android/hardware/usb/UsbDeviceConnection.java @@ -107,6 +107,34 @@ boolean isOpen() { } } + /** + * This is meant to be called by UsbRequest's queue() in order to synchronize on + * UsbDeviceConnection's mLock to prevent the connection being closed while queueing. + */ + /* package */ boolean queueRequest(UsbRequest request, ByteBuffer buffer, int length) { + synchronized (mLock) { + if (!isOpen()) { + return false; + } + + return request.queueIfConnectionOpen(buffer, length); + } + } + + /** + * This is meant to be called by UsbRequest's queue() in order to synchronize on + * UsbDeviceConnection's mLock to prevent the connection being closed while queueing. + */ + /* package */ boolean queueRequest(UsbRequest request, @Nullable ByteBuffer buffer) { + synchronized (mLock) { + if (!isOpen()) { + return false; + } + + return request.queueIfConnectionOpen(buffer); + } + } + /** * Releases all system resources related to the device. * Once the object is closed it cannot be used again. diff --git a/core/java/android/hardware/usb/UsbRequest.java b/core/java/android/hardware/usb/UsbRequest.java index d1c6465d62c85..7cabd8562e5d3 100644 --- a/core/java/android/hardware/usb/UsbRequest.java +++ b/core/java/android/hardware/usb/UsbRequest.java @@ -113,11 +113,13 @@ public boolean initialize(UsbDeviceConnection connection, UsbEndpoint endpoint) * Releases all resources related to this request. */ public void close() { - if (mNativeContext != 0) { - mEndpoint = null; - mConnection = null; - native_close(); - mCloseGuard.close(); + synchronized (mLock) { + if (mNativeContext != 0) { + mEndpoint = null; + mConnection = null; + native_close(); + mCloseGuard.close(); + } } } @@ -191,10 +193,32 @@ public void setClientData(Object data) { */ @Deprecated public boolean queue(ByteBuffer buffer, int length) { + UsbDeviceConnection connection = mConnection; + if (connection == null) { + // The expected exception by CTS Verifier - USB Device test + throw new NullPointerException("invalid connection"); + } + + // Calling into the underlying UsbDeviceConnection to synchronize on its lock, to prevent + // the connection being closed while queueing. + return connection.queueRequest(this, buffer, length); + } + + /** + * This is meant to be called from UsbDeviceConnection after synchronizing using the lock over + * there, to prevent the connection being closed while queueing. + */ + /* package */ boolean queueIfConnectionOpen(ByteBuffer buffer, int length) { + UsbDeviceConnection connection = mConnection; + if (connection == null || !connection.isOpen()) { + // The expected exception by CTS Verifier - USB Device test + throw new NullPointerException("invalid connection"); + } + boolean out = (mEndpoint.getDirection() == UsbConstants.USB_DIR_OUT); boolean result; - if (mConnection.getContext().getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.P + if (connection.getContext().getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.P && length > MAX_USBFS_BUFFER_SIZE) { length = MAX_USBFS_BUFFER_SIZE; } @@ -243,6 +267,28 @@ public boolean queue(ByteBuffer buffer, int length) { * @return true if the queueing operation succeeded */ public boolean queue(@Nullable ByteBuffer buffer) { + UsbDeviceConnection connection = mConnection; + if (connection == null) { + // The expected exception by CTS Verifier - USB Device test + throw new IllegalStateException("invalid connection"); + } + + // Calling into the underlying UsbDeviceConnection to synchronize on its lock, to prevent + // the connection being closed while queueing. + return connection.queueRequest(this, buffer); + } + + /** + * This is meant to be called from UsbDeviceConnection after synchronizing using the lock over + * there, to prevent the connection being closed while queueing. + */ + /* package */ boolean queueIfConnectionOpen(@Nullable ByteBuffer buffer) { + UsbDeviceConnection connection = mConnection; + if (connection == null || !connection.isOpen()) { + // The expected exception by CTS Verifier - USB Device test + throw new IllegalStateException("invalid connection"); + } + // Request need to be initialized Preconditions.checkState(mNativeContext != 0, "request is not initialized"); @@ -260,7 +306,7 @@ public boolean queue(@Nullable ByteBuffer buffer) { mIsUsingNewQueue = true; wasQueued = native_queue(null, 0, 0); } else { - if (mConnection.getContext().getApplicationInfo().targetSdkVersion + if (connection.getContext().getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.P) { // Can only send/receive MAX_USBFS_BUFFER_SIZE bytes at once Preconditions.checkArgumentInRange(buffer.remaining(), 0, MAX_USBFS_BUFFER_SIZE, @@ -363,11 +409,12 @@ public boolean queue(@Nullable ByteBuffer buffer) { * @return true if cancelling succeeded */ public boolean cancel() { - if (mConnection == null) { + UsbDeviceConnection connection = mConnection; + if (connection == null) { return false; } - return mConnection.cancelRequest(this); + return connection.cancelRequest(this); } /** @@ -382,7 +429,8 @@ public boolean cancel() { * @return true if cancelling succeeded. */ /* package */ boolean cancelIfOpen() { - if (mNativeContext == 0 || (mConnection != null && !mConnection.isOpen())) { + UsbDeviceConnection connection = mConnection; + if (mNativeContext == 0 || (connection != null && !connection.isOpen())) { Log.w(TAG, "Detected attempt to cancel a request on a connection which isn't open"); return false; From 71f251ba9c608e4756f18ccfb07e9bb0edf35d72 Mon Sep 17 00:00:00 2001 From: Liahav Eitan Date: Tue, 11 Oct 2022 13:20:52 +0000 Subject: [PATCH 40/42] Fix sharing to another profile where an app has multiple targets Moves the fixUris call from onTargetSelected directly to the intent launch to ensure the intent which is actually started is updated with userId specific URIs. This is a backport of ag/19657256 and ag/20063949. Bug:242165528 Bug:244876518 Bug:242605257 Test: manually share image from personal profile to work gmail, first with chat target then backing up and selecting the main target Test: manually share image from work Photos app to personal WhatsApp's frequent contact target. Change-Id: Id815984e691bf962e19e30a54f7247d16060b3b8 Merged-In: Id815984e691bf962e19e30a54f7247d16060b3b8 Merged-In: Ib41c8a3c46afcc2d62a4c1a924212bcd98bcfbe4 Merged-In: Iabf5dcf2612fe718f2f0886e2e5e9b76f37af1e1 (cherry picked from commit f50ced5f1e619d7fa7858748d6a9dbe861354f04) Merged-In: Id815984e691bf962e19e30a54f7247d16060b3b8 --- .../com/android/internal/app/ResolverActivity.java | 12 ------------ .../internal/app/chooser/DisplayResolveInfo.java | 2 ++ .../internal/app/chooser/SelectableTargetInfo.java | 1 + .../com/android/internal/app/chooser/TargetInfo.java | 11 +++++++++++ 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/core/java/com/android/internal/app/ResolverActivity.java b/core/java/com/android/internal/app/ResolverActivity.java index d08f21c9f257a..63ea936d15108 100644 --- a/core/java/com/android/internal/app/ResolverActivity.java +++ b/core/java/com/android/internal/app/ResolverActivity.java @@ -1227,9 +1227,6 @@ protected boolean onTargetSelected(TargetInfo target, boolean always) { } if (target != null) { - if (intent != null && isLaunchingTargetInOtherProfile()) { - prepareIntentForCrossProfileLaunch(intent); - } safelyStartActivity(target); // Rely on the ActivityManager to pop up a dialog regarding app suspension @@ -1242,15 +1239,6 @@ protected boolean onTargetSelected(TargetInfo target, boolean always) { return true; } - private void prepareIntentForCrossProfileLaunch(Intent intent) { - intent.fixUris(UserHandle.myUserId()); - } - - private boolean isLaunchingTargetInOtherProfile() { - return mMultiProfilePagerAdapter.getCurrentUserHandle().getIdentifier() - != UserHandle.myUserId(); - } - @VisibleForTesting public void safelyStartActivity(TargetInfo cti) { // We're dispatching intents that might be coming from legacy apps, so diff --git a/core/java/com/android/internal/app/chooser/DisplayResolveInfo.java b/core/java/com/android/internal/app/chooser/DisplayResolveInfo.java index b00148af312f0..f8e59369d0bc1 100644 --- a/core/java/com/android/internal/app/chooser/DisplayResolveInfo.java +++ b/core/java/com/android/internal/app/chooser/DisplayResolveInfo.java @@ -181,6 +181,7 @@ public boolean startAsCaller(ResolverActivity activity, Bundle options, int user if (ENABLE_CHOOSER_DELEGATE) { return activity.startAsCallerImpl(mResolvedIntent, options, false, userId); } else { + TargetInfo.prepareIntentForCrossProfileLaunch(mResolvedIntent, userId); activity.startActivityAsCaller(mResolvedIntent, options, null, false, userId); return true; } @@ -188,6 +189,7 @@ public boolean startAsCaller(ResolverActivity activity, Bundle options, int user @Override public boolean startAsUser(Activity activity, Bundle options, UserHandle user) { + TargetInfo.prepareIntentForCrossProfileLaunch(mResolvedIntent, user.getIdentifier()); activity.startActivityAsUser(mResolvedIntent, options, user); return false; } diff --git a/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java b/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java index 068b882eb4f7e..bf217ea6d27bd 100644 --- a/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java +++ b/core/java/com/android/internal/app/chooser/SelectableTargetInfo.java @@ -229,6 +229,7 @@ public boolean startAsCaller(ResolverActivity activity, Bundle options, int user } intent.setComponent(mChooserTarget.getComponentName()); intent.putExtras(mChooserTarget.getIntentExtras()); + TargetInfo.prepareIntentForCrossProfileLaunch(intent, userId); // Important: we will ignore the target security checks in ActivityManager // if and only if the ChooserTarget's target package is the same package diff --git a/core/java/com/android/internal/app/chooser/TargetInfo.java b/core/java/com/android/internal/app/chooser/TargetInfo.java index f56ab17cb0597..7bb7ddc65c6db 100644 --- a/core/java/com/android/internal/app/chooser/TargetInfo.java +++ b/core/java/com/android/internal/app/chooser/TargetInfo.java @@ -130,4 +130,15 @@ public interface TargetInfo { * @return true if this target should be pinned to the front by the request of the user */ boolean isPinned(); + + /** + * Fix the URIs in {@code intent} if cross-profile sharing is required. This should be called + * before launching the intent as another user. + */ + static void prepareIntentForCrossProfileLaunch(Intent intent, int targetUserId) { + final int currentUserId = UserHandle.myUserId(); + if (targetUserId != currentUserId) { + intent.fixUris(currentUserId); + } + } } From fed72b20d0200d8a79ee434fd32ed2c98abe4eed Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Tue, 18 Oct 2022 05:21:30 +0000 Subject: [PATCH 41/42] Ensure that only SysUI can override pending intent launch flags - Originally added in ag/5139951, this method ensured that activities launched from widgets are always started in a new task (if the activity is launched in the home task, the task is not brough forward with the recents transition). We can restrict this to only recents callers since this only applies to 1p launchers in gesture nav (both the gesture with 3p launchers and button nav in general will always start the home intent directly, which makes adding the NEW_TASK flag unnecessary). Bug: 243794108 Test: Ensure that the original bug b/112508020 still works (with the test app in the bug, swipe up still works after launching an activity from the widget, and fails without applying the override flags) Change-Id: Id53c6a2aa6da5933d488ca06a0bfc4ef89a4c343 (cherry picked from commit c4d3106e347922610f8c554de3ae238175ed393e) Merged-In: Id53c6a2aa6da5933d488ca06a0bfc4ef89a4c343 --- .../com/android/server/am/PendingIntentRecord.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/am/PendingIntentRecord.java b/services/core/java/com/android/server/am/PendingIntentRecord.java index dc924c11f867d..33f745ef211fb 100644 --- a/services/core/java/com/android/server/am/PendingIntentRecord.java +++ b/services/core/java/com/android/server/am/PendingIntentRecord.java @@ -350,11 +350,16 @@ public int sendInner(int code, Intent intent, String resolvedType, IBinder allow resolvedType = key.requestResolvedType; } - // Apply any launch flags from the ActivityOptions. This is to ensure that the caller - // can specify a consistent launch mode even if the PendingIntent is immutable + // Apply any launch flags from the ActivityOptions. This is used only by SystemUI + // to ensure that we can launch the pending intent with a consistent launch mode even + // if the provided PendingIntent is immutable (ie. to force an activity to launch into + // a new task, or to launch multiple instances if supported by the app) final ActivityOptions opts = ActivityOptions.fromBundle(options); if (opts != null) { - finalIntent.addFlags(opts.getPendingIntentLaunchFlags()); + // TODO(b/254490217): Move this check into SafeActivityOptions + if (controller.mAtmInternal.isCallerRecents(Binder.getCallingUid())) { + finalIntent.addFlags(opts.getPendingIntentLaunchFlags()); + } } // Extract options before clearing calling identity From 02a4ef7d659491feb9d6fdff52e5169ee1a55980 Mon Sep 17 00:00:00 2001 From: Joey Huab Date: Thu, 19 Jan 2023 18:04:27 +0700 Subject: [PATCH 42/42] PixelPropsUtils: Update fingerprints to January 2023 release Signed-off-by: Rahmad Adi Pratama --- .../internal/util/custom/PixelPropsUtils.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/java/com/android/internal/util/custom/PixelPropsUtils.java b/core/java/com/android/internal/util/custom/PixelPropsUtils.java index e239640a2a547..4435d1bd52495 100644 --- a/core/java/com/android/internal/util/custom/PixelPropsUtils.java +++ b/core/java/com/android/internal/util/custom/PixelPropsUtils.java @@ -35,7 +35,7 @@ public class PixelPropsUtils { private static final String TAG = PixelPropsUtils.class.getSimpleName(); private static final boolean DEBUG = false; - private static final Map propsToChangePixel6; + private static final Map propsToChangePixel6Pro; private static final Map propsToChangePixel5; private static final String[] packagesToChangePixel5 = { @@ -132,20 +132,20 @@ public class PixelPropsUtils { static { propsToKeep = new HashMap<>(); propsToKeep.put("com.google.android.settings.intelligence", new ArrayList<>(Collections.singletonList("FINGERPRINT"))); - propsToChangePixel6 = new HashMap<>(); - propsToChangePixel6.put("BRAND", "google"); - propsToChangePixel6.put("MANUFACTURER", "Google"); - propsToChangePixel6.put("DEVICE", "raven"); - propsToChangePixel6.put("PRODUCT", "raven"); - propsToChangePixel6.put("MODEL", "Pixel 6 Pro"); - propsToChangePixel6.put("FINGERPRINT", "google/raven/raven:13/TP1A.220905.004/8927612:user/release-keys"); + propsToChangePixel6Pro = new HashMap<>(); + propsToChangePixel6Pro.put("BRAND", "google"); + propsToChangePixel6Pro.put("MANUFACTURER", "Google"); + propsToChangePixel6Pro.put("DEVICE", "raven"); + propsToChangePixel6Pro.put("PRODUCT", "raven"); + propsToChangePixel6Pro.put("MODEL", "Pixel 6 Pro"); + propsToChangePixel6Pro.put("FINGERPRINT", "google/raven/raven:13/TQ1A.230105.002/9325679:user/release-keys"); propsToChangePixel5 = new HashMap<>(); propsToChangePixel5.put("BRAND", "google"); propsToChangePixel5.put("MANUFACTURER", "Google"); propsToChangePixel5.put("DEVICE", "redfin"); propsToChangePixel5.put("PRODUCT", "redfin"); propsToChangePixel5.put("MODEL", "Pixel 5"); - propsToChangePixel5.put("FINGERPRINT", "google/redfin/redfin:13/TP1A.220624.014/8819323:user/release-keys"); + propsToChangePixel5.put("FINGERPRINT", "google/redfin/redfin:13/TQ1A.230105.001/9292298:user/release-keys"); propsToChangePixelXL = new HashMap<>(); propsToChangePixelXL.put("BRAND", "google"); propsToChangePixelXL.put("MANUFACTURER", "Google"); @@ -189,7 +189,7 @@ public static void setProps(String packageName) { return; } - Map propsToChange = propsToChangePixel6; + Map propsToChange = propsToChangePixel6Pro; if (Arrays.asList(packagesToChangePixel5).contains(packageName)) { propsToChange = propsToChangePixel5; }