From ceb90c636ce0d766f80cfcd522bb27b37e6d37c9 Mon Sep 17 00:00:00 2001
From: Hall Liu <hallliu@google.com>
Date: Thu, 20 Aug 2020 19:06:57 -0700
Subject: Fix exported broadcast receiver vulnerability

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
Merged-In: I03c8163c22a6fbc92613ca2ccd2ac191fc0084a4
(cherry picked from commit e514bc6a01fdd36a519fd4fefffa45f166911c97)
(cherry picked from commit db3208b69c1debcc0746df99a4c7cab02c3c52f0)
(cherry picked from commit e2a3f5f51bf6723d54599e5fa81577f6b9465116)
---
 AndroidManifest.xml                                |  5 ++
 AndroidManifest_Platform.xml                       |  5 ++
 .../CellBroadcastAlertService.java                 | 13 ++---
 .../CellBroadcastInternalReceiver.java             | 56 +++++++++++++++++++
 .../CellBroadcastReceiver.java                     |  6 ++-
 .../unit/CellBroadcastInternalReceiverTest.java    | 63 ++++++++++++++++++++++
 .../unit/CellBroadcastReceiverTest.java            |  9 ----
 7 files changed, 140 insertions(+), 17 deletions(-)
 create mode 100644 src/com/android/cellbroadcastreceiver/CellBroadcastInternalReceiver.java
 create mode 100644 tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastInternalReceiverTest.java

diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index d3c6ebe8c..0c01772e3 100644
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -140,6 +140,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="com.android.cellbroadcastreceiver.CellBroadcastSearchIndexableProvider"
                 android:authorities="com.android.cellbroadcastreceiver"
diff --git a/AndroidManifest_Platform.xml b/AndroidManifest_Platform.xml
index d7f44e696..d9463eecd 100644
--- a/AndroidManifest_Platform.xml
+++ b/AndroidManifest_Platform.xml
@@ -135,6 +135,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 53c32a601..ee92eee77 100644
--- a/src/com/android/cellbroadcastreceiver/CellBroadcastAlertService.java
+++ b/src/com/android/cellbroadcastreceiver/CellBroadcastAlertService.java
@@ -615,9 +615,11 @@ public class CellBroadcastAlertService extends Service
                 (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
         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.getReceivedTime());
         } else {
@@ -630,7 +632,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,
@@ -661,7 +663,7 @@ public class CellBroadcastAlertService extends Service
                         .setVisibility(Notification.VISIBILITY_PUBLIC)
                         .setOngoing(nonSwipeableNotification);
 
-        if (context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_WATCH)) {
+        if (isWatch) {
             builder.setDeleteIntent(pi);
             // FEATURE_WATCH/CWH devices see this as priority
             builder.setVibrate(new long[]{0});
@@ -691,8 +693,7 @@ public class CellBroadcastAlertService extends Service
         // Emergency messages use a different audio playback and display path. Since we use
         // addToNotification for the emergency display on FEATURE WATCH devices vs the
         // Alert Dialog, it will call this and override the emergency audio tone.
-        if (context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_WATCH)
-                && !channelManager.isEmergencyMessage(message)) {
+        if (isWatch && !channelManager.isEmergencyMessage(message)) {
             if (res.getBoolean(R.bool.watch_enable_non_emergency_audio)) {
                 // start audio/vibration/speech service for non emergency alerts
                 Intent audioIntent = new Intent(context, CellBroadcastAlertAudio.class);
@@ -750,7 +751,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 b02f75c7d..0bc1501fb 100644
--- a/src/com/android/cellbroadcastreceiver/CellBroadcastReceiver.java
+++ b/src/com/android/cellbroadcastreceiver/CellBroadcastReceiver.java
@@ -41,6 +41,7 @@ import android.telephony.SubscriptionManager;
 import android.telephony.TelephonyManager;
 import android.telephony.cdma.CdmaSmsCbProgramData;
 import android.text.TextUtils;
+import android.util.EventLog;
 import android.util.Log;
 import android.widget.Toast;
 
@@ -122,8 +123,9 @@ public class CellBroadcastReceiver extends BroadcastReceiver {
         Resources res = getResourcesMethod();
 
         if (ACTION_MARK_AS_READ.equals(action)) {
-            final long deliveryTime = intent.getLongExtra(EXTRA_DELIVERY_TIME, -1);
-            getCellBroadcastTask(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 (CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED.equals(action)) {
             initializeSharedPreference();
             enableLauncher();
diff --git a/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastInternalReceiverTest.java b/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastInternalReceiverTest.java
new file mode 100644
index 000000000..713b91b8e
--- /dev/null
+++ b/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastInternalReceiverTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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.unit;
+
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.nullable;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+import android.content.Context;
+import android.content.Intent;
+import android.content.res.Configuration;
+
+import com.android.cellbroadcastreceiver.CellBroadcastInternalReceiver;
+import com.android.cellbroadcastreceiver.CellBroadcastReceiver;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+public class CellBroadcastInternalReceiverTest extends CellBroadcastTest {
+
+    @Mock Intent mIntent;
+
+    private Configuration mConfiguration = new Configuration();
+    private CellBroadcastInternalReceiver mReceiver;
+
+    @Before
+    public void setUp() throws Exception {
+        super.setUp(this.getClass().getSimpleName());
+        MockitoAnnotations.initMocks(this);
+        doReturn(mConfiguration).when(mResources).getConfiguration();
+        mReceiver = spy(new CellBroadcastInternalReceiver());
+        doReturn(mContext).when(mContext).getApplicationContext();
+    }
+
+    @Test
+    public void testOnReceive_actionMarkAsRead() {
+        doReturn(CellBroadcastReceiver.ACTION_MARK_AS_READ).when(mIntent).getAction();
+        doNothing().when(mReceiver).getCellBroadcastTask(nullable(Context.class), anyLong());
+        mReceiver.onReceive(mContext, mIntent);
+        verify(mIntent).getLongExtra(CellBroadcastReceiver.EXTRA_DELIVERY_TIME, -1);
+        verify(mReceiver).getCellBroadcastTask(nullable(Context.class), anyLong());
+    }
+
+}
diff --git a/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastReceiverTest.java b/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastReceiverTest.java
index 41a7043c1..7d81af933 100644
--- a/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastReceiverTest.java
+++ b/tests/unit/src/com/android/cellbroadcastreceiver/unit/CellBroadcastReceiverTest.java
@@ -124,15 +124,6 @@ public class CellBroadcastReceiverTest extends CellBroadcastTest {
         doReturn(mPackageName).when(mContext).getPackageName();
     }
 
-    @Test
-    public void testOnReceive_actionMarkAsRead() {
-        doReturn(CellBroadcastReceiver.ACTION_MARK_AS_READ).when(mIntent).getAction();
-        doNothing().when(mCellBroadcastReceiver).getCellBroadcastTask(anyLong());
-        mCellBroadcastReceiver.onReceive(mContext, mIntent);
-        verify(mIntent).getLongExtra(CellBroadcastReceiver.EXTRA_DELIVERY_TIME, -1);
-        verify(mCellBroadcastReceiver).getCellBroadcastTask(anyLong());
-    }
-
     @Test
     public void testOnReceive_actionCarrierConfigChanged() {
         doReturn(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED).when(mIntent).getAction();
-- 
cgit v1.2.3