From 50324497d097b9a4d37f925d9cb314e29733aab5 Mon Sep 17 00:00:00 2001 From: Jacob Hobbie Date: Thu, 24 Jun 2021 16:27:58 -0700 Subject: [PATCH] Making runtime receivers safer by default Bug: 161145287 Test: Manually verified receiver was not exported without the new flag Change-Id: Ib36a7413e96433a07bbac0f38588547e079fba11 --- core/api/current.txt | 2 + core/java/android/content/Context.java | 32 +++++++++--- .../server/am/ActivityManagerService.java | 50 ++++++++++++++++++- .../android/server/am/BroadcastFilter.java | 5 +- .../com/android/server/am/BroadcastQueue.java | 16 ++++++ 5 files changed, 96 insertions(+), 9 deletions(-) diff --git a/core/api/current.txt b/core/api/current.txt index e4e5a1acc420e..1dcbd0f7474c3 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -10681,6 +10681,8 @@ package android.content { field public static final String PERFORMANCE_HINT_SERVICE = "performance_hint"; field public static final String POWER_SERVICE = "power"; field public static final String PRINT_SERVICE = "print"; + field public static final int RECEIVER_EXPORTED = 2; // 0x2 + field public static final int RECEIVER_NOT_EXPORTED = 4; // 0x4 field public static final int RECEIVER_VISIBLE_TO_INSTANT_APPS = 1; // 0x1 field public static final String RESTRICTIONS_SERVICE = "restrictions"; field public static final String ROLE_SERVICE = "role"; diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index ad1163ff01f7f..f5a0c431f4901 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -534,8 +534,8 @@ public abstract class Context { | Context.BIND_NOT_PERCEPTIBLE | Context.BIND_NOT_VISIBLE; /** @hide */ - @IntDef(flag = true, prefix = { "RECEIVER_VISIBLE_" }, value = { - RECEIVER_VISIBLE_TO_INSTANT_APPS + @IntDef(flag = true, prefix = { "RECEIVER_VISIBLE" }, value = { + RECEIVER_VISIBLE_TO_INSTANT_APPS, RECEIVER_EXPORTED, RECEIVER_NOT_EXPORTED }) @Retention(RetentionPolicy.SOURCE) public @interface RegisterReceiverFlags {} @@ -545,6 +545,18 @@ public abstract class Context { */ public static final int RECEIVER_VISIBLE_TO_INSTANT_APPS = 0x1; + /** + * Flag for {@link #registerReceiver}: The receiver can receive broadcasts from other Apps. + * Has the same behavior as marking a statically registered receiver with "exported=true" + */ + public static final int RECEIVER_EXPORTED = 0x2; + + /** + * Flag for {@link #registerReceiver}: The receiver cannot receive broadcasts from other Apps. + * Has the same behavior as marking a statically registered receiver with "exported=false" + */ + public static final int RECEIVER_NOT_EXPORTED = 0x4; + /** * Returns an AssetManager instance for the application's package. *

@@ -2989,8 +3001,12 @@ public abstract class Context { * * @param receiver The BroadcastReceiver to handle the broadcast. * @param filter Selects the Intent broadcasts to be received. - * @param flags Additional options for the receiver. May be 0 or - * {@link #RECEIVER_VISIBLE_TO_INSTANT_APPS}. + * @param flags Additional options for the receiver. As of + * {@link android.os.Build.VERSION_CODES#TIRAMISU}, either {@link #RECEIVER_EXPORTED} or + * {@link #RECEIVER_NOT_EXPORTED} must be specified if the receiver isn't being registered + * for protected broadcasts, and may additionally specify + * {@link #RECEIVER_VISIBLE_TO_INSTANT_APPS} if {@link #RECEIVER_EXPORTED} is + * specified. * * @return The first sticky intent found that matches filter, * or null if there are none. @@ -3062,8 +3078,12 @@ public abstract class Context { * no permission is required. * @param scheduler Handler identifying the thread that will receive * the Intent. If null, the main thread of the process will be used. - * @param flags Additional options for the receiver. May be 0 or - * {@link #RECEIVER_VISIBLE_TO_INSTANT_APPS}. + * @param flags Additional options for the receiver. As of + * {@link android.os.Build.VERSION_CODES#TIRAMISU}, either {@link #RECEIVER_EXPORTED} or + * {@link #RECEIVER_NOT_EXPORTED} must be specified if the receiver isn't being registered + * for protected broadcasts, and may additionally specify + * {@link #RECEIVER_VISIBLE_TO_INSTANT_APPS} if {@link #RECEIVER_EXPORTED} is + * specified. * * @return The first sticky intent found that matches filter, * or null if there are none. diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index b202c237da47b..18cc15497b81a 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -194,6 +194,9 @@ import android.app.usage.UsageStatsManager; import android.app.usage.UsageStatsManagerInternal; import android.appwidget.AppWidgetManager; import android.appwidget.AppWidgetManagerInternal; +import android.compat.Compatibility; +import android.compat.annotation.ChangeId; +import android.compat.annotation.EnabledSince; import android.content.AttributionSource; import android.content.AutofillOptions; import android.content.BroadcastReceiver; @@ -543,6 +546,16 @@ public class ActivityManagerService extends IActivityManager.Stub static final String EXTRA_DESCRIPTION = "android.intent.extra.DESCRIPTION"; static final String EXTRA_BUGREPORT_TYPE = "android.intent.extra.BUGREPORT_TYPE"; + /** + * It is now required for apps to explicitly set either + * {@link android.content.Context#RECEIVER_EXPORTED} or + * {@link android.content.Context#RECEIVER_NOT_EXPORTED} when registering a receiver for an + * unprotected broadcast in code. + */ + @ChangeId + @EnabledSince(targetSdkVersion = Build.VERSION_CODES.TIRAMISU) + private static final long DYNAMIC_RECEIVER_EXPLICIT_EXPORT_REQUIRED = 161145287L; + /** * The maximum number of bytes that {@link #setProcessStateSummary} accepts. * @@ -12416,6 +12429,11 @@ public class ActivityManagerService extends IActivityManager.Stub ProcessRecord callerApp = null; final boolean visibleToInstantApps = (flags & Context.RECEIVER_VISIBLE_TO_INSTANT_APPS) != 0; + // Dynamic receivers are exported by default for versions prior to T + final boolean exported = + ((flags & Context.RECEIVER_EXPORTED) != 0 + || (!Compatibility.isChangeEnabled(161145287))); + int callingUid; int callingPid; boolean instantApp; @@ -12452,8 +12470,10 @@ public class ActivityManagerService extends IActivityManager.Stub noAction.add(null); actions = noAction.iterator(); } + boolean onlyProtectedBroadcasts = actions.hasNext(); - // Collect stickies of users + // Collect stickies of users and check if broadcast is only registered for protected + // broadcasts int[] userIds = { UserHandle.USER_ALL, UserHandle.getUserId(callingUid) }; while (actions.hasNext()) { String action = actions.next(); @@ -12469,6 +12489,31 @@ public class ActivityManagerService extends IActivityManager.Stub } } } + if (onlyProtectedBroadcasts) { + try { + onlyProtectedBroadcasts &= + AppGlobals.getPackageManager().isProtectedBroadcast(action); + } catch (RemoteException e) { + onlyProtectedBroadcasts = false; + Slog.w(TAG, "Remote exception", e); + } + } + } + + // If the change is enabled, but neither exported or not exported is set, we need to log + // an error so the consumer can know to explicitly set the value for their flag + if (!onlyProtectedBroadcasts && (Compatibility.isChangeEnabled(161145287) + && (flags & (Context.RECEIVER_EXPORTED | Context.RECEIVER_NOT_EXPORTED)) + == 0)) { + Slog.e(TAG, + callerPackage + ": Targeting T+ (version " + Build.VERSION_CODES.TIRAMISU + + " and above) requires that one of RECEIVER_EXPORTED or " + + "RECEIVER_NOT_EXPORTED be specified when registering a receiver"); + } else if (((flags & Context.RECEIVER_EXPORTED) != 0) && ( + (flags & Context.RECEIVER_NOT_EXPORTED) != 0)) { + throw new IllegalArgumentException( + "Receiver can't specify both RECEIVER_EXPORTED and RECEIVER_NOT_EXPORTED" + + "flag"); } } @@ -12557,7 +12602,8 @@ public class ActivityManagerService extends IActivityManager.Stub + " callerPackage is " + callerPackage); } BroadcastFilter bf = new BroadcastFilter(filter, rl, callerPackage, callerFeatureId, - receiverId, permission, callingUid, userId, instantApp, visibleToInstantApps); + receiverId, permission, callingUid, userId, instantApp, visibleToInstantApps, + exported); if (rl.containsFilter(filter)) { Slog.w(TAG, "Receiver with filter " + filter + " already registered for pid " + rl.pid diff --git a/services/core/java/com/android/server/am/BroadcastFilter.java b/services/core/java/com/android/server/am/BroadcastFilter.java index 50e06c30e17d7..8e38f0a97640a 100644 --- a/services/core/java/com/android/server/am/BroadcastFilter.java +++ b/services/core/java/com/android/server/am/BroadcastFilter.java @@ -34,10 +34,12 @@ final class BroadcastFilter extends IntentFilter { final int owningUserId; final boolean instantApp; final boolean visibleToInstantApp; + final boolean exported; BroadcastFilter(IntentFilter _filter, ReceiverList _receiverList, String _packageName, String _featureId, String _receiverId, String _requiredPermission, - int _owningUid, int _userId, boolean _instantApp, boolean _visibleToInstantApp) { + int _owningUid, int _userId, boolean _instantApp, boolean _visibleToInstantApp, + boolean _exported) { super(_filter); receiverList = _receiverList; packageName = _packageName; @@ -48,6 +50,7 @@ final class BroadcastFilter extends IntentFilter { owningUserId = _userId; instantApp = _instantApp; visibleToInstantApp = _visibleToInstantApp; + exported = _exported; } public void dumpDebug(ProtoOutputStream proto, long fieldId) { diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 503b3a93b31f8..32d93ab5903b3 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -775,6 +775,22 @@ public final class BroadcastQueue { skip = true; } + // Ensure that broadcasts are only sent to other apps if they are explicitly marked as + // exported, or are System level broadcasts + if (!skip && !filter.exported && Process.SYSTEM_UID != r.callingUid + && filter.receiverList.uid != r.callingUid) { + + Slog.w(TAG, "Exported Denial: sending " + + r.intent.toString() + + ", action: " + r.intent.getAction() + + " from " + r.callerPackage + + " (uid=" + r.callingUid + ")" + + " due to receiver " + filter.receiverList.app + + " (uid " + filter.receiverList.uid + ")" + + " not specifying RECEIVER_EXPORTED"); + // skip = true; + } + if (skip) { r.delivery[index] = BroadcastRecord.DELIVERY_SKIPPED; return;