Preventing accidental misuse of registerReceiver

A new BroadcastFilter was being created even when the caller called
registerReceiver with the same receiver and intent filters as an earlier
call. While resolving, only the first object added was being used.
Also, limiting the number of receivers that an app can register, so
badly written code does not end up causing memory or computational
pressure on the system.

Test: Manual, and
atest android.content.BroadcastReceiverTests#testReceiverLimit

Bug: 70677313
Change-Id: I04342b94d00cab3451aca4c884e15039448a760a

core/tests/coretests/src/android/content/BroadcastReceiverTests.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2018 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 android.content;
+
+import static org.junit.Assert.fail;
+
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class BroadcastReceiverTests {
+
+    private static final int RECEIVER_LIMIT_PER_APP = 1000;
+    private static final class EmptyReceiver extends BroadcastReceiver {
+        @Override
+        public void onReceive(Context context, Intent intent) {
+            // Empty
+        }
+    }
+    private Context mContext;
+
+    @Before
+    public void setUp() {
+        mContext = InstrumentationRegistry.getTargetContext();
+    }
+
+    @Test
+    public void testReceiverLimit() {
+        final IntentFilter mockFilter = new IntentFilter("android.content.tests.TestAction");
+        try {
+            for (int i = 0; i < RECEIVER_LIMIT_PER_APP + 1; i++) {
+                mContext.registerReceiver(new EmptyReceiver(), mockFilter);
+            }
+            fail("No exception thrown when registering "
+                    + (RECEIVER_LIMIT_PER_APP + 1) + " receivers");
+        } catch (IllegalStateException ise) {
+            // Expected
+        }
+    }
+}

services/core/java/com/android/server/IntentResolver.java

@@ -72,7 +72,7 @@ public abstract class IntentResolver<F extends IntentFilter, R extends Object> {
         }
     }
 
-    private boolean filterEquals(IntentFilter f1, IntentFilter f2) {
+    public static boolean filterEquals(IntentFilter f1, IntentFilter f2) {
         int s1 = f1.countActions();
         int s2 = f2.countActions();
         if (s1 != s2) {

services/core/java/com/android/server/am/ActivityManagerService.java

@@ -558,6 +558,9 @@ public class ActivityManagerService extends IActivityManager.Stub
 
     static final String SYSTEM_DEBUGGABLE = "ro.debuggable";
 
+    // Maximum number of receivers an app can register.
+    private static final int MAX_RECEIVERS_ALLOWED_PER_APP = 1000;
+
     // Amount of time after a call to stopAppSwitches() during which we will
     // prevent further untrusted switches from happening.
     static final long APP_SWITCH_DELAY_TIME = 5*1000;
@@ -20539,6 +20542,12 @@ public class ActivityManagerService extends IActivityManager.Stub
                 rl = new ReceiverList(this, callerApp, callingPid, callingUid,
                         userId, receiver);
                 if (rl.app != null) {
+                    final int totalReceiversForApp = rl.app.receivers.size();
+                    if (totalReceiversForApp >= MAX_RECEIVERS_ALLOWED_PER_APP) {
+                        throw new IllegalStateException("Too many receivers, total of "
+                                + totalReceiversForApp + ", registered for pid: "
+                                + rl.pid + ", callerPackage: " + callerPackage);
+                    }
                     rl.app.receivers.add(rl);
                 } else {
                     try {
@@ -20567,11 +20576,18 @@ public class ActivityManagerService extends IActivityManager.Stub
             }
             BroadcastFilter bf = new BroadcastFilter(filter, rl, callerPackage,
                     permission, callingUid, userId, instantApp, visibleToInstantApps);
-            rl.add(bf);
-            if (!bf.debugCheck()) {
-                Slog.w(TAG, "==> For Dynamic broadcast");
+            if (rl.containsFilter(filter)) {
+                // STOPSHIP: To track if apps are doing this a lot for b/70677313. Change to Slog.w
+                Slog.wtf(TAG, "Receiver with filter " + filter
+                        + " already registered for pid " + rl.pid
+                        + ", callerPackage is " + callerPackage);
+            } else {
+                rl.add(bf);
+                if (!bf.debugCheck()) {
+                    Slog.w(TAG, "==> For Dynamic broadcast");
+                }
+                mReceiverResolver.addFilter(bf);
             }
-            mReceiverResolver.addFilter(bf);
 
             // Enqueue broadcasts for all existing stickies that match
             // this filter.

services/core/java/com/android/server/am/ReceiverList.java

@@ -17,11 +17,14 @@
 package com.android.server.am;
 
 import android.content.IIntentReceiver;
+import android.content.IntentFilter;
 import android.os.Binder;
 import android.os.IBinder;
 import android.util.PrintWriterPrinter;
 import android.util.Printer;
 import android.util.proto.ProtoOutputStream;
+
+import com.android.server.IntentResolver;
 import com.android.server.am.proto.ReceiverListProto;
 
 import java.io.PrintWriter;
@@ -67,6 +70,17 @@ final class ReceiverList extends ArrayList<BroadcastFilter>
         owner.unregisterReceiver(receiver);
     }
 
+    public boolean containsFilter(IntentFilter filter) {
+        final int N = size();
+        for (int i = 0; i < N; i++) {
+            final BroadcastFilter f = get(i);
+            if (IntentResolver.filterEquals(f, filter)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     void writeToProto(ProtoOutputStream proto, long fieldId) {
         long token = proto.start(fieldId);
         app.writeToProto(proto, ReceiverListProto.APP);