Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add StrictMode check for unsafe intent launching

Browse Source 
Exported components that are not guarded by a signature permission
can receive Intents from any other app on a device. If an app unparcels
and launches an Intent from the Intent delivered to this unprotected
component then a malicious actor can potentially craft an Intent that
could launch hidden components, grant URI permissions, etc. This
commit adds a StrictMode check to report if a component launches an
Intent unparceled from the delivered Intent.

Bug: 160796858
Test: atest StrictModeTest
Change-Id: I763b8a965f91f5b433ce2f4b619e10ef12f5c296
This commit is contained in:
Jeff Sharkey
2021-01-15 20:53:21 -07:00
committed by Michael Groover
parent fd763b652d
commit 6728d4f92c
8 changed files with 258 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
core/api/current.txt
View File

@@ -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 {
}

83
core/java/android/app/ActivityThread.java
View File

@@ -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<WeakReference<AssistStructure>> mLastAssistStructures = new ArrayList<>();
private int mLastSessionId;
final ArrayMap<IBinder, CreateServiceData> mServicesData = new ArrayMap<>();
@UnsupportedAppUsage
final ArrayMap<IBinder, Service> 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; i<N; i++) {
ReferrerIntent intent = intents.get(i);
intent.setExtrasClassLoader(r.activity.getClassLoader());
intent.prepareToEnterProcess();
intent.prepareToEnterProcess(isProtectedComponent(r.activityInfo));
r.activity.mFragments.noteStateNotSaved();
mInstrumentation.callActivityOnNewIntent(r.activity, intent);
}
@@ -4052,7 +4055,8 @@ public final class ActivityThread extends ClientTransactionHandler {
}
java.lang.ClassLoader cl = context.getClassLoader();
data.intent.setExtrasClassLoader(cl);
data.intent.prepareToEnterProcess();
data.intent.prepareToEnterProcess(
isProtectedComponent(data.info) || isProtectedBroadcast(data.intent));
data.setExtrasClassLoader(cl);
receiver = packageInfo.getAppFactory()
.instantiateReceiver(cl, data.info.name, data.intent);
@@ -4249,6 +4253,7 @@ public final class ActivityThread extends ClientTransactionHandler {
service.attach(context, this, data.info.name, data.token, app,
ActivityManager.getService());
service.onCreate();
mServicesData.put(data.token, data);
mServices.put(data.token, service);
try {
ActivityManager.getService().serviceDoneExecuting(
@@ -4266,13 +4271,14 @@ public final class ActivityThread extends ClientTransactionHandler {
}
private void handleBindService(BindServiceData data) {
CreateServiceData createData = mServicesData.get(data.token);
Service s = mServices.get(data.token);
if (DEBUG_SERVICE)
Slog.v(TAG, "handleBindService s=" + s + " rebind=" + data.rebind);
if (s != null) {
try {
data.intent.setExtrasClassLoader(s.getClassLoader());
data.intent.prepareToEnterProcess();
data.intent.prepareToEnterProcess(isProtectedComponent(createData.info));
try {
if (!data.rebind) {
IBinder binder = s.onBind(data.intent);
@@ -4297,11 +4303,12 @@ public final class ActivityThread extends ClientTransactionHandler {
}
private void handleUnbindService(BindServiceData data) {
CreateServiceData createData = mServicesData.get(data.token);
Service s = mServices.get(data.token);
if (s != null) {
try {
data.intent.setExtrasClassLoader(s.getClassLoader());
data.intent.prepareToEnterProcess();
data.intent.prepareToEnterProcess(isProtectedComponent(createData.info));
boolean doRebind = s.onUnbind(data.intent);
try {
if (doRebind) {
@@ -4373,12 +4380,13 @@ public final class ActivityThread extends ClientTransactionHandler {
}
private void handleServiceArgs(ServiceArgsData data) {
CreateServiceData createData = mServicesData.get(data.token);
Service s = mServices.get(data.token);
if (s != null) {
try {
if (data.args != null) {
data.args.setExtrasClassLoader(s.getClassLoader());
data.args.prepareToEnterProcess();
data.args.prepareToEnterProcess(isProtectedComponent(createData.info));
}
int res;
if (!data.taskRemoved) {
@@ -4407,6 +4415,7 @@ public final class ActivityThread extends ClientTransactionHandler {
}
private void handleStopService(IBinder token) {
mServicesData.remove(token);
Service s = mServices.remove(token);
if (s != null) {
try {
@@ -5026,7 +5035,7 @@ public final class ActivityThread extends ClientTransactionHandler {
try {
if (ri.mData != null) {
ri.mData.setExtrasClassLoader(r.activity.getClassLoader());
ri.mData.prepareToEnterProcess();
ri.mData.prepareToEnterProcess(isProtectedComponent(r.activityInfo));
}
if (DEBUG_RESULTS) Slog.v(TAG,
"Delivering result to activity " + r + " : " + ri);
@@ -7739,6 +7748,66 @@ public final class ActivityThread extends ClientTransactionHandler {
Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);
}
/**
* Returns whether the provided {@link ActivityInfo} {@code ai} is a protected component.
*
* @see #isProtectedComponent(ComponentInfo, String)
*/
public static boolean isProtectedComponent(@NonNull ActivityInfo ai) {
return isProtectedComponent(ai, ai.permission);
}
/**
* Returns whether the provided {@link ServiceInfo} {@code si} is a protected component.
*
* @see #isProtectedComponent(ComponentInfo, String)
*/
public static boolean isProtectedComponent(@NonNull ServiceInfo si) {
return isProtectedComponent(si, si.permission);
}
/**
* Returns whether the provided {@link ComponentInfo} {@code ci} with the specified {@code
* permission} is a protected component.
*
* <p>A 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);

4
core/java/android/app/ContextImpl.java
View File

@@ -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) {

4
core/java/android/app/LoadedApk.java
View File

@@ -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);

4
core/java/android/content/ClipData.java
View File

@@ -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);
}
}
}

46
core/java/android/content/Intent.java
View File

@@ -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 <em>cannot</em> 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<String> 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 */

71
core/java/android/os/StrictMode.java
View File

@@ -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 {
* <p>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.
* <p>
* 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:
* <ul>
* <li>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.
* <li>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.
* </ul>
* <p>
* Note that this <em>may</em> 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.
* <p>
* Disabling this check is <em>strongly discouraged</em>, 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;

53
core/java/android/os/strictmode/UnsafeIntentLaunchViolation.java Normal file
View File

@@ -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.
* <p>
* 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:
* <ul>
* <li>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.
* <li>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.
* </ul>
* <p>
* Note that this <em>may</em> 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);
}
}