diff options
| author | Hall Liu <hallliu@google.com> | 2020-08-20 19:06:57 -0700 |
|---|---|---|
| committer | Anis Assi <anisassi@google.com> | 2020-09-10 13:50:16 -0700 |
| commit | 13fee8e7f3f47e3ae492d709f8d550b4a2588626 (patch) | |
| tree | e809c99d27a0785c92b497787738e219d41dd848 | |
| parent | 41cd9596220e1635ff8c0ab14e8ce1a3b228d1ba (diff) | |
| download | platform_packages_apps_CellBroadcastReceiver-oreo-mr1-security-release.tar.gz platform_packages_apps_CellBroadcastReceiver-oreo-mr1-security-release.tar.bz2 platform_packages_apps_CellBroadcastReceiver-oreo-mr1-security-release.zip | |
RESTRICT AUTOMERGE Fix exported broadcast receiver vulnerabilityandroid-security-8.1.0_r93android-security-8.1.0_r92android-security-8.1.0_r91android-security-8.1.0_r90android-security-8.1.0_r89android-security-8.1.0_r88android-security-8.1.0_r87android-security-8.1.0_r86android-security-8.1.0_r85android-security-8.1.0_r84android-security-8.1.0_r83android-security-8.1.0_r82oreo-mr1-security-release
CellBroadcastReceiver was declared as exported in the manifest and
therefore allowed any app to send a MARK_AS_READ intent, even though
it's only supposed to be called from an internal PendingIntent. Fix this
by creating a new non-exported receiver and using that to handle the
mark-as-read intent instead.
Fixes: 162741784
Test: atest GoogleCellBroadcastReceiverUnitTests
Change-Id: I03c8163c22a6fbc92613ca2ccd2ac191fc0084a4
(cherry picked from commit e514bc6a01fdd36a519fd4fefffa45f166911c97)
(cherry picked from commit c41e7acd2c289b3dcef42d2e88d21dcac61b2f86)
4 files changed, 71 insertions, 13 deletions
diff --git a/AndroidManifest.xml b/AndroidManifest.xml index 4972533ff..cb6bacf71 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -128,6 +128,11 @@ </intent-filter> </receiver> + <receiver android:name="com.android.cellbroadcastreceiver.CellBroadcastInternalReceiver" + android:exported="false"> + <!-- No intent filter on purpose: this should only receive explicit intents --> + </receiver> + <provider android:name="CellBroadcastSearchIndexableProvider" android:authorities="com.android.cellbroadcastreceiver" diff --git a/src/com/android/cellbroadcastreceiver/CellBroadcastAlertService.java b/src/com/android/cellbroadcastreceiver/CellBroadcastAlertService.java index d80bff7ea..6694987b7 100644 --- a/src/com/android/cellbroadcastreceiver/CellBroadcastAlertService.java +++ b/src/com/android/cellbroadcastreceiver/CellBroadcastAlertService.java @@ -603,9 +603,11 @@ public class CellBroadcastAlertService extends Service { final NotificationManager notificationManager = NotificationManager.from(context); createNotificationChannels(context); + boolean isWatch = context.getPackageManager() + .hasSystemFeature(PackageManager.FEATURE_WATCH); // Create intent to show the new messages when user selects the notification. Intent intent; - if (context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_WATCH)) { + if (isWatch) { // For FEATURE_WATCH we want to mark as read intent = createMarkAsReadIntent(context, message.getDeliveryTime()); } else { @@ -618,7 +620,7 @@ public class CellBroadcastAlertService extends Service { intent.putExtra(CellBroadcastAlertDialog.FROM_SAVE_STATE_NOTIFICATION_EXTRA, fromSaveState); PendingIntent pi; - if (context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_WATCH)) { + if (isWatch) { pi = PendingIntent.getBroadcast(context, 0, intent, 0); } else { pi = PendingIntent.getActivity(context, NOTIFICATION_ID, intent, @@ -639,7 +641,7 @@ public class CellBroadcastAlertService extends Service { builder.setDefaults(Notification.DEFAULT_ALL); - if (context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_WATCH)) { + if (isWatch) { builder.setDeleteIntent(pi); } else { builder.setContentIntent(pi); @@ -687,7 +689,7 @@ public class CellBroadcastAlertService extends Service { * @return delete intent to add to the pending intent */ static Intent createMarkAsReadIntent(Context context, long deliveryTime) { - Intent deleteIntent = new Intent(context, CellBroadcastReceiver.class); + Intent deleteIntent = new Intent(context, CellBroadcastInternalReceiver.class); deleteIntent.setAction(CellBroadcastReceiver.ACTION_MARK_AS_READ); deleteIntent.putExtra(CellBroadcastReceiver.EXTRA_DELIVERY_TIME, deliveryTime); return deleteIntent; diff --git a/src/com/android/cellbroadcastreceiver/CellBroadcastInternalReceiver.java b/src/com/android/cellbroadcastreceiver/CellBroadcastInternalReceiver.java new file mode 100644 index 000000000..455c5b68c --- /dev/null +++ b/src/com/android/cellbroadcastreceiver/CellBroadcastInternalReceiver.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2020 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 com.android.cellbroadcastreceiver; + +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; +import android.provider.Telephony; + +import com.android.internal.annotations.VisibleForTesting; + +/** + * {@link BroadcastReceiver} used for handling internal broadcasts (e.g. generated from + * {@link android.app.PendingIntent}s). + */ +public class CellBroadcastInternalReceiver extends BroadcastReceiver { + + /** + * helper method for easier testing. To generate a new CellBroadcastTask + * @param deliveryTime message delivery time + */ + @VisibleForTesting + public void getCellBroadcastTask(Context context, long deliveryTime) { + new CellBroadcastContentProvider.AsyncCellBroadcastTask(context.getContentResolver()) + .execute(new CellBroadcastContentProvider.CellBroadcastOperation() { + @Override + public boolean execute(CellBroadcastContentProvider provider) { + return provider.markBroadcastRead(Telephony.CellBroadcasts.DELIVERY_TIME, + deliveryTime); + } + }); + } + + @Override + public void onReceive(Context context, Intent intent) { + if (CellBroadcastReceiver.ACTION_MARK_AS_READ.equals(intent.getAction())) { + final long deliveryTime = intent.getLongExtra( + CellBroadcastReceiver.EXTRA_DELIVERY_TIME, -1); + getCellBroadcastTask(context, deliveryTime); + } + } +} diff --git a/src/com/android/cellbroadcastreceiver/CellBroadcastReceiver.java b/src/com/android/cellbroadcastreceiver/CellBroadcastReceiver.java index 012089946..67588e7ce 100644 --- a/src/com/android/cellbroadcastreceiver/CellBroadcastReceiver.java +++ b/src/com/android/cellbroadcastreceiver/CellBroadcastReceiver.java @@ -27,6 +27,7 @@ import android.provider.Telephony; import android.provider.Telephony.CellBroadcasts; import android.telephony.CarrierConfigManager; import android.telephony.cdma.CdmaSmsCbProgramData; +import android.util.EventLog; import android.util.Log; import com.android.internal.telephony.TelephonyIntents; @@ -59,15 +60,9 @@ public class CellBroadcastReceiver extends BroadcastReceiver { String action = intent.getAction(); if (ACTION_MARK_AS_READ.equals(action)) { - final long deliveryTime = intent.getLongExtra(EXTRA_DELIVERY_TIME, -1); - new CellBroadcastContentProvider.AsyncCellBroadcastTask(context.getContentResolver()) - .execute(new CellBroadcastContentProvider.CellBroadcastOperation() { - @Override - public boolean execute(CellBroadcastContentProvider provider) { - return provider.markBroadcastRead(CellBroadcasts.DELIVERY_TIME, - deliveryTime); - } - }); + // The only way this'll be called is if someone tries to maliciously set something as + // read. Log an event. + EventLog.writeEvent(0x534e4554, "162741784", -1, null); } else if (TelephonyIntents.ACTION_DEFAULT_SMS_SUBSCRIPTION_CHANGED.equals(action) || CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED.equals(action) || Intent.ACTION_BOOT_COMPLETED.equals(action) |
