From ff78495653d181b0459d16ae0ca4269bc799b28e Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Wed, 28 Feb 2018 20:55:40 -0800 Subject: [PATCH] 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 --- .../content/BroadcastReceiverTests.java | 60 +++++++++++++++++++ .../com/android/server/IntentResolver.java | 2 +- .../server/am/ActivityManagerService.java | 24 ++++++-- .../com/android/server/am/ReceiverList.java | 14 +++++ 4 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 core/tests/coretests/src/android/content/BroadcastReceiverTests.java diff --git a/core/tests/coretests/src/android/content/BroadcastReceiverTests.java b/core/tests/coretests/src/android/content/BroadcastReceiverTests.java new file mode 100644 index 0000000000000..8deccb7ffa7fe --- /dev/null +++ b/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 + } + } +} diff --git a/services/core/java/com/android/server/IntentResolver.java b/services/core/java/com/android/server/IntentResolver.java index 119c9df6c5762..ea80ac1aecef3 100644 --- a/services/core/java/com/android/server/IntentResolver.java +++ b/services/core/java/com/android/server/IntentResolver.java @@ -72,7 +72,7 @@ public abstract class IntentResolver { } } - 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) { diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 6726b6e1cc23d..55805f8cf9a4a 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/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. diff --git a/services/core/java/com/android/server/am/ReceiverList.java b/services/core/java/com/android/server/am/ReceiverList.java index a989063134858..eee924f6b23f8 100644 --- a/services/core/java/com/android/server/am/ReceiverList.java +++ b/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 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);