From 12bb4ed9ab46d3e42326ef1c5e7b90aae80a9bfc Mon Sep 17 00:00:00 2001 From: Evan Chen Date: Tue, 17 Oct 2023 21:31:23 +0000 Subject: Do not allow setting notification access across users. For mutil user case, make sure the calling userid matching the passing userid Test: test it on sample app Bug: 298635078 Change-Id: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a Merged-In: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a --- .../android/server/companion/CompanionDeviceManagerService.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index 0a617cdd2168..5c36a6b07392 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -685,8 +685,7 @@ public class CompanionDeviceManagerService extends SystemService { public PendingIntent requestNotificationAccess(ComponentName component, int userId) throws RemoteException { String callingPackage = component.getPackageName(); - checkCanCallNotificationApi(callingPackage); - // TODO: check userId. + checkCanCallNotificationApi(callingPackage, userId); if (component.flattenToString().length() > MAX_CN_LENGTH) { throw new IllegalArgumentException("Component name is too long."); } @@ -712,7 +711,7 @@ public class CompanionDeviceManagerService extends SystemService { @Deprecated @Override public boolean hasNotificationAccess(ComponentName component) throws RemoteException { - checkCanCallNotificationApi(component.getPackageName()); + checkCanCallNotificationApi(component.getPackageName(), getCallingUserId()); NotificationManager nm = getContext().getSystemService(NotificationManager.class); return nm.isNotificationListenerAccessGranted(component); } @@ -908,8 +907,7 @@ public class CompanionDeviceManagerService extends SystemService { createNewAssociation(userId, packageName, macAddressObj, null, null, false); } - private void checkCanCallNotificationApi(String callingPackage) { - final int userId = getCallingUserId(); + private void checkCanCallNotificationApi(String callingPackage, int userId) { enforceCallerIsSystemOr(userId, callingPackage); if (getCallingUid() == SYSTEM_UID) return; -- cgit v1.2.3 From 15eec4872d7b0fdfead3a8f5b4a1bb4d9ad82a0c Mon Sep 17 00:00:00 2001 From: Evan Chen Date: Tue, 17 Oct 2023 21:31:23 +0000 Subject: Do not allow setting notification access across users. For mutil user case, make sure the calling userid matching the passing userid Test: test it on sample app Bug: 298635078 Change-Id: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a Merged-In: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a --- .../android/server/companion/CompanionDeviceManagerService.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index 3ac7b8e6668e..5f6211f158c2 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -543,8 +543,7 @@ public class CompanionDeviceManagerService extends SystemService { public PendingIntent requestNotificationAccess(ComponentName component, int userId) throws RemoteException { String callingPackage = component.getPackageName(); - checkCanCallNotificationApi(callingPackage); - // TODO: check userId. + checkCanCallNotificationApi(callingPackage, userId); if (component.flattenToString().length() > MAX_CN_LENGTH) { throw new IllegalArgumentException("Component name is too long."); } @@ -570,7 +569,7 @@ public class CompanionDeviceManagerService extends SystemService { @Deprecated @Override public boolean hasNotificationAccess(ComponentName component) throws RemoteException { - checkCanCallNotificationApi(component.getPackageName()); + checkCanCallNotificationApi(component.getPackageName(), getCallingUserId()); NotificationManager nm = getContext().getSystemService(NotificationManager.class); return nm.isNotificationListenerAccessGranted(component); } @@ -727,8 +726,7 @@ public class CompanionDeviceManagerService extends SystemService { legacyCreateAssociation(userId, macAddress, packageName, null); } - private void checkCanCallNotificationApi(String callingPackage) { - final int userId = getCallingUserId(); + private void checkCanCallNotificationApi(String callingPackage, int userId) { enforceCallerIsSystemOr(userId, callingPackage); if (getCallingUid() == SYSTEM_UID) return; -- cgit v1.2.3 From ee88e15751ff0bc3da6b7c43dc7d94cebf1fc1a2 Mon Sep 17 00:00:00 2001 From: Evan Chen Date: Tue, 17 Oct 2023 21:31:23 +0000 Subject: Do not allow setting notification access across users. For mutil user case, make sure the calling userid matching the passing userid Test: test it on sample app Bug: 298635078 Change-Id: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a Merged-In: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a --- .../android/server/companion/CompanionDeviceManagerService.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index f5ce00cb31c9..41546d2bdc38 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -620,8 +620,7 @@ public class CompanionDeviceManagerService extends SystemService { public PendingIntent requestNotificationAccess(ComponentName component, int userId) throws RemoteException { String callingPackage = component.getPackageName(); - checkCanCallNotificationApi(callingPackage); - // TODO: check userId. + checkCanCallNotificationApi(callingPackage, userId); if (component.flattenToString().length() > MAX_CN_LENGTH) { throw new IllegalArgumentException("Component name is too long."); } @@ -647,7 +646,7 @@ public class CompanionDeviceManagerService extends SystemService { @Deprecated @Override public boolean hasNotificationAccess(ComponentName component) throws RemoteException { - checkCanCallNotificationApi(component.getPackageName()); + checkCanCallNotificationApi(component.getPackageName(), getCallingUserId()); NotificationManager nm = getContext().getSystemService(NotificationManager.class); return nm.isNotificationListenerAccessGranted(component); } @@ -804,8 +803,7 @@ public class CompanionDeviceManagerService extends SystemService { legacyCreateAssociation(userId, macAddress, packageName, null); } - private void checkCanCallNotificationApi(String callingPackage) { - final int userId = getCallingUserId(); + private void checkCanCallNotificationApi(String callingPackage, int userId) { enforceCallerIsSystemOr(userId, callingPackage); if (getCallingUid() == SYSTEM_UID) return; -- cgit v1.2.3 From bf8ff047eb25960720a688cb16aa44b3775799da Mon Sep 17 00:00:00 2001 From: William Leshner Date: Wed, 1 Nov 2023 18:03:35 +0000 Subject: Fix vulnerability that allowed attackers to start arbitary activities Test: Flashed device and verified dream settings works as expected Test: Installed APK from bug and verified the dream didn't allow launching the inappropriate settings activity. Fixes: 300090204 Change-Id: I146415ad400827d0a798e27f34f098feb5e96422 Merged-In: I6e90e3a0d513dceb7d7f5c59d6807ebe164c5716 --- core/java/android/service/dreams/DreamService.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/core/java/android/service/dreams/DreamService.java b/core/java/android/service/dreams/DreamService.java index 2d461c6cf92e..d380522de643 100644 --- a/core/java/android/service/dreams/DreamService.java +++ b/core/java/android/service/dreams/DreamService.java @@ -1192,8 +1192,17 @@ public class DreamService extends Service implements Window.Callback { if (!flattenedString.contains("/")) { return new ComponentName(serviceInfo.packageName, flattenedString); } - - return ComponentName.unflattenFromString(flattenedString); + // Ensure that the component is from the same package as the dream service. If not, + // treat the component as invalid and return null instead. + final ComponentName cn = ComponentName.unflattenFromString(flattenedString); + if (cn == null) return null; + if (!cn.getPackageName().equals(serviceInfo.packageName)) { + Log.w(TAG, + "Inconsistent package name in component: " + cn.getPackageName() + + ", should be: " + serviceInfo.packageName); + return null; + } + return cn; } /** -- cgit v1.2.3 From 6926fd15fb16c51468dde270bd61ee68772b8c14 Mon Sep 17 00:00:00 2001 From: Will Leshner Date: Tue, 31 Oct 2023 13:23:08 -0700 Subject: Fix vulnerability that allowed attackers to start arbitary activities Test: Flashed device and verified dream settings works as expected Test: Installed APK from bug and verified the dream didn't allow launching the inappropriate settings activity. Fixes: 300090204 Merged-In: I6e90e3a0d513dceb7d7f5c59d6807ebe164c5716 Merged-In: I146415ad400827d0a798e27f34f098feb5e96422 Merged-In: I7f2351fc7d9a82778ce21f67018a45ac67c9aaf8 Change-Id: I573040df84bf98a493b39f96c8581e4303206bac --- .../src/com/android/settingslib/dream/DreamBackend.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/dream/DreamBackend.java b/packages/SettingsLib/src/com/android/settingslib/dream/DreamBackend.java index ab7b54d98285..beadd821957b 100644 --- a/packages/SettingsLib/src/com/android/settingslib/dream/DreamBackend.java +++ b/packages/SettingsLib/src/com/android/settingslib/dream/DreamBackend.java @@ -351,7 +351,17 @@ public class DreamBackend { if (cn != null && cn.indexOf('/') < 0) { cn = resolveInfo.serviceInfo.packageName + "/" + cn; } - return cn == null ? null : ComponentName.unflattenFromString(cn); + // Ensure that the component is from the same package as the dream service. If not, + // treat the component as invalid and return null instead. + final ComponentName result = cn != null ? ComponentName.unflattenFromString(cn) : null; + if (result != null + && !result.getPackageName().equals(resolveInfo.serviceInfo.packageName)) { + Log.w(TAG, + "Inconsistent package name in component: " + result.getPackageName() + + ", should be: " + resolveInfo.serviceInfo.packageName); + return null; + } + return result; } private static void logd(String msg, Object... args) { -- cgit v1.2.3