diff --git a/core/api/current.txt b/core/api/current.txt index 5e07790378be9..04eadaa67bc58 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -31276,6 +31276,7 @@ package android.os { method @NonNull public android.os.StrictMode.VmPolicy.Builder detectLeakedRegistrationObjects(); method @NonNull public android.os.StrictMode.VmPolicy.Builder detectLeakedSqlLiteObjects(); method @NonNull public android.os.StrictMode.VmPolicy.Builder detectNonSdkApiUsage(); + method @NonNull public android.os.StrictMode.VmPolicy.Builder detectUnsafeIntentLaunch(); method @NonNull public android.os.StrictMode.VmPolicy.Builder detectUntaggedSockets(); method @NonNull public android.os.StrictMode.VmPolicy.Builder penaltyDeath(); method @NonNull public android.os.StrictMode.VmPolicy.Builder penaltyDeathOnCleartextNetwork(); @@ -31284,6 +31285,7 @@ package android.os { method @NonNull public android.os.StrictMode.VmPolicy.Builder penaltyListener(@NonNull java.util.concurrent.Executor, @NonNull android.os.StrictMode.OnVmViolationListener); method @NonNull public android.os.StrictMode.VmPolicy.Builder penaltyLog(); method @NonNull public android.os.StrictMode.VmPolicy.Builder permitNonSdkApiUsage(); + method @NonNull public android.os.StrictMode.VmPolicy.Builder permitUnsafeIntentLaunch(); method @NonNull public android.os.StrictMode.VmPolicy.Builder setClassInstanceLimit(Class, int); } @@ -31810,6 +31812,9 @@ package android.os.strictmode { public final class UnbufferedIoViolation extends android.os.strictmode.Violation { } + public final class UnsafeIntentLaunchViolation extends android.os.strictmode.Violation { + } + public final class UntaggedSocketViolation extends android.os.strictmode.Violation { } diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index bd437f4ce1925..e5c34c58e1818 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -64,12 +64,14 @@ import android.content.IIntentReceiver; import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; +import android.content.pm.ComponentInfo; import android.content.pm.IPackageManager; import android.content.pm.InstrumentationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; import android.content.pm.ParceledListSlice; +import android.content.pm.PermissionInfo; import android.content.pm.ProviderInfo; import android.content.pm.ProviderInfoList; import android.content.pm.ServiceInfo; @@ -346,6 +348,7 @@ public final class ActivityThread extends ClientTransactionHandler { private int mPendingProcessState = PROCESS_STATE_UNKNOWN; ArrayList> mLastAssistStructures = new ArrayList<>(); private int mLastSessionId; + final ArrayMap mServicesData = new ArrayMap<>(); @UnsupportedAppUsage final ArrayMap mServices = new ArrayMap<>(); @UnsupportedAppUsage @@ -3412,7 +3415,7 @@ public final class ActivityThread extends ClientTransactionHandler { cl, component.getClassName(), r.intent); StrictMode.incrementExpectedActivityCount(activity.getClass()); r.intent.setExtrasClassLoader(cl); - r.intent.prepareToEnterProcess(); + r.intent.prepareToEnterProcess(isProtectedComponent(r.activityInfo)); if (r.state != null) { r.state.setClassLoader(cl); } @@ -3717,7 +3720,7 @@ public final class ActivityThread extends ClientTransactionHandler { for (int i=0; iA component is protected if it is not exported, or if the specified {@code permission} is + * a signature permission. + */ + private static boolean isProtectedComponent(@NonNull ComponentInfo ci, + @Nullable String permission) { + // Bail early when this process isn't looking for violations + if (!StrictMode.vmUnsafeIntentLaunchEnabled()) return false; + + // TODO: consider optimizing by having AMS pre-calculate this value + if (!ci.exported) { + return true; + } + if (permission != null) { + try { + PermissionInfo pi = getPermissionManager().getPermissionInfo(permission, + currentOpPackageName(), 0); + return (pi != null) && pi.getProtection() == PermissionInfo.PROTECTION_SIGNATURE; + } catch (RemoteException ignored) { + } + } + return false; + } + + /** + * Returns whether the action within the provided {@code intent} is a protected broadcast. + */ + public static boolean isProtectedBroadcast(@NonNull Intent intent) { + // Bail early when this process isn't looking for violations + if (!StrictMode.vmUnsafeIntentLaunchEnabled()) return false; + + // TODO: consider optimizing by having AMS pre-calculate this value + try { + return getPackageManager().isProtectedBroadcast(intent.getAction()); + } catch (RemoteException ignored) { + } + return false; + } + // ------------------ Regular JNI ------------------------ private native void nPurgePendingResources(); private native void nDumpGraphicsInfo(FileDescriptor fd); diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index bac5025387403..5222699490211 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -1670,7 +1670,9 @@ class ContextImpl extends Context { flags); if (intent != null) { intent.setExtrasClassLoader(getClassLoader()); - intent.prepareToEnterProcess(); + // TODO: determine at registration time if caller is + // protecting themselves with signature permission + intent.prepareToEnterProcess(ActivityThread.isProtectedBroadcast(intent)); } return intent; } catch (RemoteException e) { diff --git a/core/java/android/app/LoadedApk.java b/core/java/android/app/LoadedApk.java index 50853a3bcf9c2..c01b5a32b98b4 100644 --- a/core/java/android/app/LoadedApk.java +++ b/core/java/android/app/LoadedApk.java @@ -1617,7 +1617,9 @@ public final class LoadedApk { try { ClassLoader cl = mReceiver.getClass().getClassLoader(); intent.setExtrasClassLoader(cl); - intent.prepareToEnterProcess(); + // TODO: determine at registration time if caller is + // protecting themselves with signature permission + intent.prepareToEnterProcess(ActivityThread.isProtectedBroadcast(intent)); setExtrasClassLoader(cl); receiver.setPendingResult(this); receiver.onReceive(mContext, intent); diff --git a/core/java/android/content/ClipData.java b/core/java/android/content/ClipData.java index c4d98671a5984..0188637741848 100644 --- a/core/java/android/content/ClipData.java +++ b/core/java/android/content/ClipData.java @@ -1008,7 +1008,9 @@ public class ClipData implements Parcelable { for (int i = 0; i < size; i++) { final Item item = mItems.get(i); if (item.mIntent != null) { - item.mIntent.prepareToEnterProcess(); + // We can't recursively claim that this data is from a protected + // component, since it may have been filled in by a malicious app + item.mIntent.prepareToEnterProcess(false); } } } diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index 7843d97aa4110..1752b480c06b5 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -6681,6 +6681,25 @@ public class Intent implements Parcelable, Cloneable { | FLAG_GRANT_WRITE_URI_PERMISSION | FLAG_GRANT_PERSISTABLE_URI_PERMISSION | FLAG_GRANT_PREFIX_URI_PERMISSION; + /** + * Local flag indicating this instance was created by copy constructor. + */ + private static final int LOCAL_FLAG_FROM_COPY = 1 << 0; + + /** + * Local flag indicating this instance was created from a {@link Parcel}. + */ + private static final int LOCAL_FLAG_FROM_PARCEL = 1 << 1; + + /** + * Local flag indicating this instance was delivered through a protected + * component, such as an activity that requires a signature permission, or a + * protected broadcast. Note that this flag cannot be recursively + * applied to any contained instances, since a malicious app may have + * controlled them via {@link #fillIn(Intent, int)}. + */ + private static final int LOCAL_FLAG_FROM_PROTECTED_COMPONENT = 1 << 2; + // --------------------------------------------------------------------- // --------------------------------------------------------------------- // toUri() and parseUri() options. @@ -6798,6 +6817,8 @@ public class Intent implements Parcelable, Cloneable { private String mPackage; private ComponentName mComponent; private int mFlags; + /** Set of in-process flags which are never parceled */ + private int mLocalFlags; private ArraySet mCategories; @UnsupportedAppUsage private Bundle mExtras; @@ -6848,6 +6869,11 @@ public class Intent implements Parcelable, Cloneable { this.mCategories = new ArraySet<>(o.mCategories); } + // Inherit flags from the original, plus mark that we were + // created by this copy constructor + this.mLocalFlags = o.mLocalFlags; + this.mLocalFlags |= LOCAL_FLAG_FROM_COPY; + if (copyMode != COPY_MODE_FILTER) { this.mFlags = o.mFlags; this.mContentUserHint = o.mContentUserHint; @@ -10931,6 +10957,9 @@ public class Intent implements Parcelable, Cloneable { /** @hide */ protected Intent(Parcel in) { + // Remember that we came from a remote process to help detect security + // issues caused by later unsafe launches + mLocalFlags = LOCAL_FLAG_FROM_PARCEL; readFromParcel(in); } @@ -11242,18 +11271,27 @@ public class Intent implements Parcelable, Cloneable { mData = Uri.fromFile(after); } } + + // Detect cases where we're about to launch a potentially unsafe intent + if ((mLocalFlags & LOCAL_FLAG_FROM_PARCEL) != 0 + && (mLocalFlags & LOCAL_FLAG_FROM_PROTECTED_COMPONENT) == 0 + && StrictMode.vmUnsafeIntentLaunchEnabled()) { + StrictMode.onUnsafeIntentLaunch(this); + } } /** * @hide */ - public void prepareToEnterProcess() { + public void prepareToEnterProcess(boolean fromProtectedComponent) { // We just entered destination process, so we should be able to read all // parcelables inside. setDefusable(true); if (mSelector != null) { - mSelector.prepareToEnterProcess(); + // We can't recursively claim that this data is from a protected + // component, since it may have been filled in by a malicious app + mSelector.prepareToEnterProcess(false); } if (mClipData != null) { mClipData.prepareToEnterProcess(); @@ -11265,6 +11303,10 @@ public class Intent implements Parcelable, Cloneable { mContentUserHint = UserHandle.USER_CURRENT; } } + + if (fromProtectedComponent) { + mLocalFlags |= LOCAL_FLAG_FROM_PROTECTED_COMPONENT; + } } /** @hide */ diff --git a/core/java/android/os/StrictMode.java b/core/java/android/os/StrictMode.java index b13be9f4a91a3..1189fdb0b04a0 100644 --- a/core/java/android/os/StrictMode.java +++ b/core/java/android/os/StrictMode.java @@ -52,6 +52,7 @@ import android.os.strictmode.ResourceMismatchViolation; import android.os.strictmode.ServiceConnectionLeakedViolation; import android.os.strictmode.SqliteObjectLeakedViolation; import android.os.strictmode.UnbufferedIoViolation; +import android.os.strictmode.UnsafeIntentLaunchViolation; import android.os.strictmode.UntaggedSocketViolation; import android.os.strictmode.Violation; import android.os.strictmode.WebViewMethodCalledOnWrongThreadViolation; @@ -256,6 +257,7 @@ public final class StrictMode { DETECT_VM_NON_SDK_API_USAGE, DETECT_VM_IMPLICIT_DIRECT_BOOT, DETECT_VM_INCORRECT_CONTEXT_USE, + DETECT_VM_UNSAFE_INTENT_LAUNCH, PENALTY_GATHER, PENALTY_LOG, PENALTY_DIALOG, @@ -297,6 +299,8 @@ public final class StrictMode { private static final int DETECT_VM_CREDENTIAL_PROTECTED_WHILE_LOCKED = 1 << 11; /** @hide */ private static final int DETECT_VM_INCORRECT_CONTEXT_USE = 1 << 12; + /** @hide */ + private static final int DETECT_VM_UNSAFE_INTENT_LAUNCH = 1 << 13; /** @hide */ private static final int DETECT_VM_ALL = 0x0000ffff; @@ -854,6 +858,7 @@ public final class StrictMode { *

In the Honeycomb release this includes leaks of SQLite cursors, Activities, and * other closable objects but will likely expand in future releases. */ + @SuppressWarnings("AndroidFrameworkCompatChange") public @NonNull Builder detectAll() { detectLeakedSqlLiteObjects(); @@ -885,6 +890,9 @@ public final class StrictMode { if (targetSdk >= Build.VERSION_CODES.R) { detectIncorrectContextUse(); } + if (targetSdk >= Build.VERSION_CODES.S) { + detectUnsafeIntentLaunch(); + } // TODO: Decide whether to detect non SDK API usage beyond a certain API level. // TODO: enable detectImplicitDirectBoot() once system is less noisy @@ -1066,6 +1074,59 @@ public final class StrictMode { return disable(DETECT_VM_INCORRECT_CONTEXT_USE); } + /** + * Detect when your app launches an {@link Intent} which originated + * from outside your app. + *

+ * Violations may indicate security vulnerabilities in the design of + * your app, where a malicious app could trick you into granting + * {@link Uri} permissions or launching unexported components. Here + * are some typical design patterns that can be used to safely + * resolve these violations: + *

    + *
  • The ideal approach is to migrate to using a + * {@link android.app.PendingIntent}, which ensures that your launch is + * performed using the identity of the original creator, completely + * avoiding the security issues described above. + *
  • If using a {@link android.app.PendingIntent} isn't feasible, an + * alternative approach is to create a brand new {@link Intent} and + * carefully copy only specific values from the original + * {@link Intent} after careful validation. + *
+ *

+ * Note that this may detect false-positives if your app + * sends itself an {@link Intent} which is first routed through the + * OS, such as using {@link Intent#createChooser}. In these cases, + * careful inspection is required to determine if the return point + * into your app is appropriately protected with a signature + * permission or marked as unexported. If the return point is not + * protected, your app is likely vulnerable to malicious apps. + * + * @see Context#startActivity(Intent) + * @see Context#startService(Intent) + * @see Context#bindService(Intent, ServiceConnection, int) + * @see Context#sendBroadcast(Intent) + * @see android.app.Activity#setResult(int, Intent) + */ + public @NonNull Builder detectUnsafeIntentLaunch() { + return enable(DETECT_VM_UNSAFE_INTENT_LAUNCH); + } + + /** + * Permit your app to launch any {@link Intent} which originated + * from outside your app. + *

+ * Disabling this check is strongly discouraged, as + * violations may indicate security vulnerabilities in the design of + * your app, where a malicious app could trick you into granting + * {@link Uri} permissions or launching unexported components. + * + * @see #detectUnsafeIntentLaunch() + */ + public @NonNull Builder permitUnsafeIntentLaunch() { + return disable(DETECT_VM_UNSAFE_INTENT_LAUNCH); + } + /** * Crashes the whole process on violation. This penalty runs at the end of all enabled * penalties so you'll still get your logging or other violations before the process @@ -2114,6 +2175,11 @@ public final class StrictMode { return (sVmPolicy.mask & DETECT_VM_INCORRECT_CONTEXT_USE) != 0; } + /** @hide */ + public static boolean vmUnsafeIntentLaunchEnabled() { + return (sVmPolicy.mask & DETECT_VM_UNSAFE_INTENT_LAUNCH) != 0; + } + /** @hide */ public static void onSqliteObjectLeaked(String message, Throwable originStack) { onVmPolicyViolation(new SqliteObjectLeakedViolation(message, originStack)); @@ -2221,6 +2287,11 @@ public final class StrictMode { } } + /** @hide */ + public static void onUnsafeIntentLaunch(Intent intent) { + onVmPolicyViolation(new UnsafeIntentLaunchViolation(intent)); + } + /** Assume locked until we hear otherwise */ private static volatile boolean sUserKeyUnlocked = false; diff --git a/core/java/android/os/strictmode/UnsafeIntentLaunchViolation.java b/core/java/android/os/strictmode/UnsafeIntentLaunchViolation.java new file mode 100644 index 0000000000000..891fb59326df8 --- /dev/null +++ b/core/java/android/os/strictmode/UnsafeIntentLaunchViolation.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2021 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.os.strictmode; + +import android.annotation.NonNull; +import android.app.PendingIntent; +import android.content.Intent; +import android.net.Uri; + +/** + * Violation raised when your app launches an {@link Intent} which originated + * from outside your app. + *

+ * Violations may indicate security vulnerabilities in the design of your app, + * where a malicious app could trick you into granting {@link Uri} permissions + * or launching unexported components. Here are some typical design patterns + * that can be used to safely resolve these violations: + *

    + *
  • The ideal approach is to migrate to using a {@link PendingIntent}, which + * ensures that your launch is performed using the identity of the original + * creator, completely avoiding the security issues described above. + *
  • If using a {@link PendingIntent} isn't feasible, an alternative approach + * is to create a brand new {@link Intent} and carefully copy only specific + * values from the original {@link Intent} after careful validation. + *
+ *

+ * Note that this may detect false-positives if your app sends itself + * an {@link Intent} which is first routed through the OS, such as using + * {@link Intent#createChooser}. In these cases, careful inspection is required + * to determine if the return point into your app is appropriately protected + * with a signature permission or marked as unexported. If the return point is + * not protected, your app is likely vulnerable to malicious apps. + */ +public final class UnsafeIntentLaunchViolation extends Violation { + /** @hide */ + public UnsafeIntentLaunchViolation(@NonNull Intent intent) { + super("Launch of unsafe intent: " + intent); + } +}