From c31de269cc54133b4f4c88fef66b9bfd7df46967 Mon Sep 17 00:00:00 2001 From: Aishwarya Mallampati Date: Fri, 28 Oct 2022 23:39:20 +0000 Subject: [PATCH 1/2] DO NOT MERGE Grant carrier privileges if package has carrier config access. TelephonyManager#hasCarrierPrivileges internally uses SubscriptionManager#canManageSubscription to decide whether to grant carrier privilege status to an app or not. SubscriptionManager#canManageSubscription returns true if caller APK's certificate matches with one of the mNativeAccessRules or mCarrierConfigAccessRules. This over-grants carrier privilege status to apps that only has mNativeAccessRules. Carrier privilege status should be granted to the caller APK only if it's certificate matches with one of mCarrierConfigAccessRules. Replaced SubscriptionManager#canManageSubscription with PhoneInterfaceManager#hasCarrierConfigAccess which returns true only if caller APK certificates matches with one of mCarrierConfigAccessRules of the given subscription. Bug: 226593252 Test: Manual Testing as explained in b/226593252#comment51 atest CtsTelephonyTestCases Flashed build on raven-userdebug and performed basic funtionality tests (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:213aba7e18ddadf800be981b802d8e242c61e0ad) Merged-In: I6899de902e6e3ffda47b48d0ae806ac9c17ee2a6 Change-Id: I6899de902e6e3ffda47b48d0ae806ac9c17ee2a6 --- .../android/phone/PhoneInterfaceManager.java | 56 ++++++++++++++++--- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/src/com/android/phone/PhoneInterfaceManager.java b/src/com/android/phone/PhoneInterfaceManager.java index d740c8f6a..244d2143a 100755 --- a/src/com/android/phone/PhoneInterfaceManager.java +++ b/src/com/android/phone/PhoneInterfaceManager.java @@ -94,6 +94,7 @@ import android.telephony.TelephonyHistogram; import android.telephony.TelephonyManager; import android.telephony.TelephonyScanManager; +import android.telephony.UiccAccessRule; import android.telephony.UiccCardInfo; import android.telephony.UiccSlotInfo; import android.telephony.UssdResponse; @@ -5836,14 +5837,18 @@ private int getCarrierPrivilegeStatusFromCarrierConfigRules(int privilegeFromSim PackageManager pkgMgr = phone.getContext().getPackageManager(); String[] packages = pkgMgr.getPackagesForUid(uid); + if (packages == null) { + return privilegeFromSim; + } final long identity = Binder.clearCallingIdentity(); try { - SubscriptionInfo subInfo = subController.getSubscriptionInfo(phone.getSubId()); - SubscriptionManager subManager = (SubscriptionManager) - phone.getContext().getSystemService(Context.TELEPHONY_SUBSCRIPTION_SERVICE); + int subId = phone.getSubId(); + SubscriptionInfo subInfo = subController.getSubscriptionInfo(subId); + List carrierConfigAccessRules = subInfo.getCarrierConfigAccessRules(); + for (String pkg : packages) { - if (subManager.canManageSubscription(subInfo, pkg)) { + if (hasCarrierConfigAccess(pkg, pkgMgr, carrierConfigAccessRules)) { return TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS; } } @@ -5862,16 +5867,51 @@ private int getCarrierPrivilegeStatusFromCarrierConfigRules(int privilegeFromSim final long identity = Binder.clearCallingIdentity(); try { - SubscriptionInfo subInfo = subController.getSubscriptionInfo(phone.getSubId()); - SubscriptionManager subManager = (SubscriptionManager) - phone.getContext().getSystemService(Context.TELEPHONY_SUBSCRIPTION_SERVICE); - return subManager.canManageSubscription(subInfo, pkgName) + int subId = phone.getSubId(); + SubscriptionInfo subInfo = subController.getSubscriptionInfo(subId); + List carrierConfigAccessRules = subInfo.getCarrierConfigAccessRules(); + + return hasCarrierConfigAccess(pkgName, phone.getContext().getPackageManager(), + carrierConfigAccessRules) ? TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS : privilegeFromSim; } finally { Binder.restoreCallingIdentity(identity); } } + /** + * Check whether carrier privilege status can be granted to the provided app for this + * subscription based on the carrier config access rules of the subscription. + * + * @param packageName package name of the app to check + * @param packageManager package manager + * @param carrierConfigAccessRules carrier config access rules of the subscription + * @return true if the app is included in the mCarrierConfigAccessRules of this subscription. + */ + private boolean hasCarrierConfigAccess(String packageName, PackageManager packageManager, + @NonNull List carrierConfigAccessRules) { + if ((packageName == null) || (carrierConfigAccessRules.isEmpty())) { + return false; + } + + PackageInfo packageInfo; + try { + packageInfo = packageManager.getPackageInfo(packageName, + PackageManager.GET_SIGNING_CERTIFICATES); + } catch (PackageManager.NameNotFoundException e) { + logv("Unknown package: " + packageName); + return false; + } + + for (UiccAccessRule rule : carrierConfigAccessRules) { + if (rule.getCarrierPrivilegeStatus(packageInfo) + == TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS) { + return true; + } + } + return false; + } + @Override public int getCarrierPrivilegeStatus(int subId) { final Phone phone = getPhone(subId); From 40563c05bcafd3f33941a15a637c7bd4023cfeb2 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Date: Fri, 26 May 2023 14:18:46 +0000 Subject: [PATCH 2/2] RESTRICT AUTOMERGE Fixed leak of cross user data in multiple settings. - Any app is allowed to receive GET_CONTENT intent. Using this, an user puts back in the intent an uri with data of another user. - Telephony service has INTERACT_ACROSS_USER permission. Using this, it reads and shows the deta to the evil user. Fix: When telephony service gets the intent result, it checks if the uri is from the current user or not. Bug: b/256591023 , b/256819787 Test: The malicious behaviour was not being reproduced. Unable to import contact from other users data. Test2: Able to import contact from the primary user or uri with no user id (These settings are not available for secondary users) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:36e10a6d0d7b9efc543f8004729fa85751f4f70d) Merged-In: I1e3a643f17948153aecc1d0df9ffd9619ad678c1 Change-Id: I1e3a643f17948153aecc1d0df9ffd9619ad678c1 --- .../android/phone/GsmUmtsCallForwardOptions.java | 12 ++++++++++++ .../phone/settings/VoicemailSettingsActivity.java | 14 ++++++++++++++ .../phone/settings/fdn/EditFdnContactScreen.java | 13 +++++++++++++ 3 files changed, 39 insertions(+) diff --git a/src/com/android/phone/GsmUmtsCallForwardOptions.java b/src/com/android/phone/GsmUmtsCallForwardOptions.java index fda0ea526..db830deb6 100644 --- a/src/com/android/phone/GsmUmtsCallForwardOptions.java +++ b/src/com/android/phone/GsmUmtsCallForwardOptions.java @@ -1,10 +1,13 @@ package com.android.phone; import android.app.ActionBar; +import android.content.ContentProvider; import android.content.Intent; import android.database.Cursor; import android.os.Bundle; import android.os.PersistableBundle; +import android.os.Process; +import android.os.UserHandle; import android.preference.Preference; import android.preference.PreferenceScreen; import android.telephony.CarrierConfigManager; @@ -203,6 +206,15 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) { } Cursor cursor = null; try { + // check if the URI returned by the user belongs to the user + final int currentUser = UserHandle.getUserId(Process.myUid()); + if (currentUser + != ContentProvider.getUserIdFromUri(data.getData(), currentUser)) { + + Log.w(LOG_TAG, "onActivityResult: Contact data of different user, " + + "cannot access"); + return; + } cursor = getContentResolver().query(data.getData(), NUM_PROJECTION, null, null, null); if ((cursor == null) || (!cursor.moveToFirst())) { diff --git a/src/com/android/phone/settings/VoicemailSettingsActivity.java b/src/com/android/phone/settings/VoicemailSettingsActivity.java index 66b1af96f..df8ecf5cb 100644 --- a/src/com/android/phone/settings/VoicemailSettingsActivity.java +++ b/src/com/android/phone/settings/VoicemailSettingsActivity.java @@ -17,6 +17,7 @@ package com.android.phone.settings; import android.app.Dialog; +import android.content.ContentProvider; import android.content.DialogInterface; import android.content.Intent; import android.database.Cursor; @@ -25,6 +26,8 @@ import android.os.Handler; import android.os.Message; import android.os.PersistableBundle; +import android.os.Process; +import android.os.UserHandle; import android.os.UserManager; import android.preference.Preference; import android.preference.PreferenceActivity; @@ -518,6 +521,17 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) { Cursor cursor = null; try { + // check if the URI returned by the user belongs to the user + final int currentUser = UserHandle.getUserId(Process.myUid()); + if (currentUser + != ContentProvider.getUserIdFromUri(data.getData(), currentUser)) { + + if (DBG) { + log("onActivityResult: Contact data of different user, " + + "cannot access"); + } + return; + } cursor = getContentResolver().query(data.getData(), new String[] { CommonDataKinds.Phone.NUMBER }, null, null, null); if ((cursor == null) || (!cursor.moveToFirst())) { diff --git a/src/com/android/phone/settings/fdn/EditFdnContactScreen.java b/src/com/android/phone/settings/fdn/EditFdnContactScreen.java index 140cc74c6..edb9f8eff 100644 --- a/src/com/android/phone/settings/fdn/EditFdnContactScreen.java +++ b/src/com/android/phone/settings/fdn/EditFdnContactScreen.java @@ -18,9 +18,12 @@ import static android.view.Window.PROGRESS_VISIBILITY_OFF; import static android.view.Window.PROGRESS_VISIBILITY_ON; +import static android.app.Activity.RESULT_OK; + import android.app.Activity; import android.content.AsyncQueryHandler; +import android.content.ContentProvider; import android.content.ContentResolver; import android.content.ContentValues; import android.content.Intent; @@ -30,6 +33,8 @@ import android.os.Bundle; import android.os.Handler; import android.os.PersistableBundle; +import android.os.Process; +import android.os.UserHandle; import android.provider.ContactsContract.CommonDataKinds; import android.telephony.PhoneNumberUtils; import android.text.Editable; @@ -166,6 +171,14 @@ protected void onActivityResult(int requestCode, int resultCode, Intent intent) } Cursor cursor = null; try { + // check if the URI returned by the user belongs to the user + final int currentUser = UserHandle.getUserId(Process.myUid()); + if (currentUser + != ContentProvider.getUserIdFromUri(intent.getData(), currentUser)) { + Log.w(LOG_TAG, "onActivityResult: Contact data of different user, " + + "cannot access"); + return; + } cursor = getContentResolver().query(intent.getData(), NUM_PROJECTION, null, null, null); if ((cursor == null) || (!cursor.moveToFirst())) {