From ce3216cbde1a2f0a7912f027aeb0c30316613116 Mon Sep 17 00:00:00 2001 From: Grace Jia Date: Thu, 22 Sep 2022 14:20:57 -0700 Subject: Fix security vulnerability when register phone accounts. Currently if the registered self-managed phone account updated to a call provider phone account, the enable state will be directly copied to the updated one so that malicious app can perform call spoofing attack without any permission requirements. Fix this by disallowing change a self-managed phone account to a managed phone account. Bug: 246930197 Test: CtsTelecomTestCases:SelfManagedConnectionSreviceTest Change-Id: I8f7984cd491632b3219133044438b82ca4dec80e Merged-In: I8f7984cd491632b3219133044438b82ca4dec80e --- src/com/android/server/telecom/PhoneAccountRegistrar.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/com/android/server/telecom/PhoneAccountRegistrar.java b/src/com/android/server/telecom/PhoneAccountRegistrar.java index ff7c03148..19949f59f 100644 --- a/src/com/android/server/telecom/PhoneAccountRegistrar.java +++ b/src/com/android/server/telecom/PhoneAccountRegistrar.java @@ -50,6 +50,7 @@ import android.telephony.TelephonyManager; import android.text.TextUtils; import android.util.AtomicFile; import android.util.Base64; +import android.util.EventLog; import android.util.Xml; // TODO: Needed for move to system service: import com.android.internal.R; @@ -818,6 +819,7 @@ public class PhoneAccountRegistrar { PhoneAccount oldAccount = getPhoneAccountUnchecked(account.getAccountHandle()); if (oldAccount != null) { + enforceSelfManagedAccountUnmodified(account, oldAccount); mState.accounts.remove(oldAccount); isEnabled = oldAccount.isEnabled(); Log.i(this, "Modify account: %s", getAccountDiffString(account, oldAccount)); @@ -878,6 +880,19 @@ public class PhoneAccountRegistrar { } } + private void enforceSelfManagedAccountUnmodified(PhoneAccount newAccount, + PhoneAccount oldAccount) { + if (oldAccount.hasCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED) && + (!newAccount.hasCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED))) { + EventLog.writeEvent(0x534e4554, "246930197"); + Log.w(this, "Self-managed phone account %s replaced by a non self-managed one", + newAccount.getAccountHandle()); + throw new IllegalArgumentException("Error, cannot change a self-managed " + + "phone account " + newAccount.getAccountHandle() + + " to other kinds of phone account"); + } + } + /** * Un-registers all phone accounts associated with a specified package. * -- cgit v1.2.3 From 833dd8480adc773e36d388521a14fd8cd11d6a30 Mon Sep 17 00:00:00 2001 From: Grace Jia Date: Thu, 22 Sep 2022 14:20:57 -0700 Subject: Fix security vulnerability when register phone accounts. Currently if the registered self-managed phone account updated to a call provider phone account, the enable state will be directly copied to the updated one so that malicious app can perform call spoofing attack without any permission requirements. Fix this by disallowing change a self-managed phone account to a managed phone account. Bug: 246930197 Test: CtsTelecomTestCases:SelfManagedConnectionSreviceTest Change-Id: I8f7984cd491632b3219133044438b82ca4dec80e Merged-In: I8f7984cd491632b3219133044438b82ca4dec80e --- src/com/android/server/telecom/PhoneAccountRegistrar.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/com/android/server/telecom/PhoneAccountRegistrar.java b/src/com/android/server/telecom/PhoneAccountRegistrar.java index 16eaa97c5..13b176c25 100644 --- a/src/com/android/server/telecom/PhoneAccountRegistrar.java +++ b/src/com/android/server/telecom/PhoneAccountRegistrar.java @@ -49,6 +49,7 @@ import android.telephony.TelephonyManager; import android.text.TextUtils; import android.util.AtomicFile; import android.util.Base64; +import android.util.EventLog; import android.util.Xml; // TODO: Needed for move to system service: import com.android.internal.R; @@ -787,6 +788,7 @@ public class PhoneAccountRegistrar { PhoneAccount oldAccount = getPhoneAccountUnchecked(account.getAccountHandle()); if (oldAccount != null) { + enforceSelfManagedAccountUnmodified(account, oldAccount); mState.accounts.remove(oldAccount); isEnabled = oldAccount.isEnabled(); Log.i(this, "Modify account: %s", getAccountDiffString(account, oldAccount)); @@ -847,6 +849,19 @@ public class PhoneAccountRegistrar { } } + private void enforceSelfManagedAccountUnmodified(PhoneAccount newAccount, + PhoneAccount oldAccount) { + if (oldAccount.hasCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED) && + (!newAccount.hasCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED))) { + EventLog.writeEvent(0x534e4554, "246930197"); + Log.w(this, "Self-managed phone account %s replaced by a non self-managed one", + newAccount.getAccountHandle()); + throw new IllegalArgumentException("Error, cannot change a self-managed " + + "phone account " + newAccount.getAccountHandle() + + " to other kinds of phone account"); + } + } + /** * Un-registers all phone accounts associated with a specified package. * -- cgit v1.2.3 From d6e319ed7d239242a43ee09fd42d5cb05593042f Mon Sep 17 00:00:00 2001 From: Bill Yi Date: Wed, 16 Nov 2022 02:13:41 -0800 Subject: Import translations. DO NOT MERGE ANYWHERE Auto-generated-cl: translation import Change-Id: Idce9e9f850c4f18c2d97ae9219fb9b5b3ce0c5ec --- res/values-en-rCA/strings.xml | 24 ++++++++++++------------ res/values-es-rUS/strings.xml | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/res/values-en-rCA/strings.xml b/res/values-en-rCA/strings.xml index cef6db56d..2e2f05f3a 100644 --- a/res/values-en-rCA/strings.xml +++ b/res/values-en-rCA/strings.xml @@ -16,7 +16,7 @@ - "Phone calls" + "Phone Calls" "Phone" "Unknown" "Missed call" @@ -35,8 +35,8 @@ "Your call used the phone app that came with your device" "Call muted." "Speakerphone enabled." - "I am so sorry, I can\'t answer the phone right now. How can I help you?" - "I\'ll call you back." + "Can\'t talk now. What\'s up?" + "I\'ll call you right back." "I\'ll call you later." "Can\'t talk now. Call me later?" "Quick responses" @@ -47,7 +47,7 @@ "Message failed to send to %s." "Calling accounts" "Only emergency calls are allowed." - "This application cannot make outgoing calls without Phone permission." + "This application cannot make outgoing calls without the Phone permission." "To place a call, enter a valid number." "Call cannot be added at this time." "Missing voicemail number" @@ -56,10 +56,10 @@ "Make %s your default Phone app?" "Set Default" "Cancel" - "%s will be able to place and control all aspects of calls. Only apps that you trust should be set as the default Phone app." + "%s will be able to place and control all aspects of calls. Only apps you trust should be set as the default Phone app." "Make %s your default call screening app?" "%s will no longer be able to screen calls." - "%s will be able to see information about callers not in your contacts and will be able to block these calls. Only apps that you trust should be set as the default call screening app." + "%s will be able to see information about callers not in your contacts and will be able to block these calls. Only apps you trust should be set as the default call screening app." "Set Default" "Cancel" "Blocked numbers" @@ -73,13 +73,13 @@ "Only the device owner can view and manage blocked numbers." "Unblock" "Blocking temporarily off" - "When you dial or text an emergency number, blocking is turned off to ensure that emergency services can contact you." + "After you dial or text an emergency number, blocking is turned off to ensure that emergency services can contact you." "Re-enable now" "%1$s blocked" "%1$s unblocked" "Unable to block emergency number." "%1$s is already blocked." - "Using the personal dialler to make the call" + "Using the personal dialer to make the call" "%1$s call from %2$s" "%1$s video call from %2$s" "Answering will end your %1$s call" @@ -90,7 +90,7 @@ "Answering will end your ongoing video call" "Answer" "Decline" - "Call cannot be placed because there are no calling accounts that support calls of this type." + "Call cannot be placed because there are no calling accounts which support calls of this type." "Call cannot be placed due to your %1$s call." "Call cannot be placed due to your %1$s calls." "Call cannot be placed due to a call in another app." @@ -101,7 +101,7 @@ "Disconnected calls" "Crashed phone apps" "Placing this call will end your %1$s call." - "Choose how to make this call" + "Choose how to place this call" "Redirect call using %1$s" "Call using my phone number" "Call can\'t be placed by %1$s. Try using a different call redirecting app or contacting the developer for help." @@ -109,8 +109,8 @@ "Numbers not in Contacts" "Block numbers that are not listed in your Contacts" "Private" - "Block callers who do not disclose their number" - "Phonebox" + "Block callers that do not disclose their number" + "Pay phone" "Block calls from pay phones" "Unknown" "Block calls from unidentified callers" diff --git a/res/values-es-rUS/strings.xml b/res/values-es-rUS/strings.xml index ac0b22c65..d2a1f4c95 100644 --- a/res/values-es-rUS/strings.xml +++ b/res/values-es-rUS/strings.xml @@ -34,7 +34,7 @@ "%s dejó de responder" "Tu llamada se hizo con la app de teléfono que venía en tu dispositivo" "Llamada silenciada" - "Altavoz habilitado" + "Bocina habilitada" "No puedo hablar ahora. ¿Todo bien?" "Te llamo enseguida." "Te llamo más tarde." -- cgit v1.2.3 From 799347b6f65da163673b4f8358783f3087ff08a2 Mon Sep 17 00:00:00 2001 From: Grace Jia Date: Thu, 25 Aug 2022 14:47:19 -0700 Subject: Fix security vulnerability issue for multi user call redirections. Currently we won't check if the PhoneAccountHandle provided by a CallRedirectionService has multi-user capability or belong to the same user as the current user. Add the check and disconnect the call if this is an unexpected cross-user call redirection. Bug: 235098883 Test: CallsManagerTest, manual test with test app provided in b/235098883. Change-Id: Ia8b9468aa2bb8e3157c227e2617ff6a52e0af119 Merged-In: Ia8b9468aa2bb8e3157c227e2617ff6a52e0af119 (cherry picked from commit f29ab7e1ec0e480e2d39d289d5aa3fc95aed2142) (cherry picked from commit 735b84a90e5305915836329d4abca672fcc87126) --- src/com/android/server/telecom/CallsManager.java | 19 ++++++++++++---- .../server/telecom/tests/CallsManagerTest.java | 25 +++++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java index 4d829bb86..73cdd5b10 100755 --- a/src/com/android/server/telecom/CallsManager.java +++ b/src/com/android/server/telecom/CallsManager.java @@ -2091,6 +2091,16 @@ public class CallsManager extends Call.ListenerBase boolean endEarly = false; String disconnectReason = ""; String callRedirectionApp = mRoleManagerAdapter.getDefaultCallRedirectionApp(); + PhoneAccount phoneAccount = mPhoneAccountRegistrar + .getPhoneAccountUnchecked(phoneAccountHandle); + if (phoneAccount != null + && !phoneAccount.hasCapabilities(PhoneAccount.CAPABILITY_MULTI_USER)) { + // Check if the phoneAccountHandle belongs to the current user + if (phoneAccountHandle != null && + !phoneAccountHandle.getUserHandle().equals(call.getInitiatingUser())) { + phoneAccountHandle = null; + } + } boolean isPotentialEmergencyNumber; try { @@ -2125,9 +2135,9 @@ public class CallsManager extends Call.ListenerBase endEarly = true; disconnectReason = "Null handle from Call Redirection Service"; } else if (phoneAccountHandle == null) { - Log.w(this, "onCallRedirectionComplete: phoneAccountHandle is null"); + Log.w(this, "onCallRedirectionComplete: phoneAccountHandle is unavailable"); endEarly = true; - disconnectReason = "Null phoneAccountHandle from Call Redirection Service"; + disconnectReason = "Unavailable phoneAccountHandle from Call Redirection Service"; } else if (isPotentialEmergencyNumber) { Log.w(this, "onCallRedirectionComplete: emergency number %s is redirected from Call" + " Redirection Service", handle.getSchemeSpecificPart()); @@ -2148,6 +2158,7 @@ public class CallsManager extends Call.ListenerBase return; } + final PhoneAccountHandle finalPhoneAccountHandle = phoneAccountHandle; if (uiAction.equals(CallRedirectionProcessor.UI_TYPE_USER_DEFINED_ASK_FOR_CONFIRM)) { Log.addEvent(call, LogUtils.Events.REDIRECTION_USER_CONFIRMATION); mPendingRedirectedOutgoingCall = call; @@ -2157,7 +2168,7 @@ public class CallsManager extends Call.ListenerBase @Override public void loggedRun() { Log.addEvent(call, LogUtils.Events.REDIRECTION_USER_CONFIRMED); - call.setTargetPhoneAccount(phoneAccountHandle); + call.setTargetPhoneAccount(finalPhoneAccountHandle); placeOutgoingCall(call, handle, gatewayInfo, speakerphoneOn, videoState); } @@ -2167,7 +2178,7 @@ public class CallsManager extends Call.ListenerBase new Runnable("CM.oCRC", mLock) { @Override public void loggedRun() { - call.setTargetPhoneAccount(phoneAccountHandle); + call.setTargetPhoneAccount(finalPhoneAccountHandle); placeOutgoingCall(call, handle, null, speakerphoneOn, videoState); } diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java index 6fd8334b6..e7d0a2641 100644 --- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java +++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java @@ -37,7 +37,6 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -57,7 +56,7 @@ import android.os.UserHandle; import android.telecom.CallerInfo; import android.telecom.Connection; import android.telecom.DisconnectCause; -import android.telecom.Log; +import android.telecom.GatewayInfo; import android.telecom.PhoneAccount; import android.telecom.PhoneAccountHandle; import android.telecom.TelecomManager; @@ -77,7 +76,6 @@ import com.android.server.telecom.CallDiagnosticServiceController; import com.android.server.telecom.CallState; import com.android.server.telecom.CallerInfoLookupHelper; import com.android.server.telecom.CallsManager; -import com.android.server.telecom.CallsManagerListenerBase; import com.android.server.telecom.ClockProxy; import com.android.server.telecom.ConnectionServiceFocusManager; import com.android.server.telecom.ConnectionServiceFocusManager.ConnectionServiceFocusManagerFactory; @@ -133,8 +131,12 @@ import java.util.concurrent.TimeUnit; @RunWith(JUnit4.class) public class CallsManagerTest extends TelecomTestCase { private static final int TEST_TIMEOUT = 5000; // milliseconds + private static final int SECONDARY_USER_ID = 12; private static final PhoneAccountHandle SIM_1_HANDLE = new PhoneAccountHandle( ComponentName.unflattenFromString("com.foo/.Blah"), "Sim1"); + private static final PhoneAccountHandle SIM_1_HANDLE_SECONDARY = new PhoneAccountHandle( + ComponentName.unflattenFromString("com.foo/.Blah"), "Sim1", + new UserHandle(SECONDARY_USER_ID)); private static final PhoneAccountHandle SIM_2_HANDLE = new PhoneAccountHandle( ComponentName.unflattenFromString("com.foo/.Blah"), "Sim2"); private static final PhoneAccountHandle CONNECTION_MGR_1_HANDLE = new PhoneAccountHandle( @@ -1676,6 +1678,23 @@ public class CallsManagerTest extends TelecomTestCase { new UserHandle(90210))); } + @SmallTest + @Test + public void testCrossUserCallRedirectionEndEarlyForIncapablePhoneAccount() { + when(mPhoneAccountRegistrar.getPhoneAccountUnchecked(eq(SIM_1_HANDLE_SECONDARY))) + .thenReturn(SIM_1_ACCOUNT); + mCallsManager.onUserSwitch(UserHandle.SYSTEM); + + Call callSpy = addSpyCall(CallState.NEW); + mCallsManager.onCallRedirectionComplete(callSpy, TEST_ADDRESS, SIM_1_HANDLE_SECONDARY, + new GatewayInfo("foo", TEST_ADDRESS2, TEST_ADDRESS), true /* speakerphoneOn */, + VideoProfile.STATE_AUDIO_ONLY, false /* shouldCancelCall */, "" /* uiAction */); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(String.class); + verify(callSpy).disconnect(argumentCaptor.capture()); + assertTrue(argumentCaptor.getValue().contains("Unavailable phoneAccountHandle")); + } + private Call addSpyCall() { return addSpyCall(SIM_2_HANDLE, CallState.ACTIVE); } -- cgit v1.2.3