Abort or throw SecurityException if intent redirect detected
Consolidate wtf logs for intent redirect prevention and actually
abort the start or throw SecurityException.
Bug: 368559093
Test: manual test
Flag: android.security.prevent_intent_redirect_abort_or_throw_exception
Change-Id: I2960fbe99ec7ea8417cf90e32a9194e6f0377574
diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java
index f719528..f798523 100644
--- a/core/java/android/content/Intent.java
+++ b/core/java/android/content/Intent.java
@@ -40,7 +40,10 @@
import android.app.ActivityThread;
import android.app.AppGlobals;
import android.app.StatusBarManager;
+import android.app.compat.CompatChanges;
import android.bluetooth.BluetoothDevice;
+import android.compat.annotation.ChangeId;
+import android.compat.annotation.Overridable;
import android.compat.annotation.UnsupportedAppUsage;
import android.content.pm.ActivityInfo;
import android.content.pm.ApplicationInfo;
@@ -670,6 +673,11 @@
public class Intent implements Parcelable, Cloneable {
private static final String TAG = "Intent";
+ /** @hide */
+ @ChangeId
+ @Overridable
+ public static final long ENABLE_PREVENT_INTENT_REDIRECT = 29076063L;
+
private static final String ATTR_ACTION = "action";
private static final String TAG_CATEGORIES = "categories";
private static final String ATTR_CATEGORY = "category";
@@ -12240,7 +12248,7 @@
* @hide
*/
public void collectExtraIntentKeys() {
- if (!preventIntentRedirect()) return;
+ if (!isPreventIntentRedirectEnabled()) return;
if (mExtras != null && !mExtras.isParcelled() && !mExtras.isEmpty()) {
for (String key : mExtras.keySet()) {
@@ -12257,6 +12265,14 @@
}
}
+ /**
+ * @hide
+ */
+ public static boolean isPreventIntentRedirectEnabled() {
+ return preventIntentRedirect() && CompatChanges.isChangeEnabled(
+ ENABLE_PREVENT_INTENT_REDIRECT);
+ }
+
/** @hide */
public void checkCreatorToken() {
if (mExtras == null) return;
@@ -12331,7 +12347,7 @@
out.writeInt(0);
}
- if (preventIntentRedirect()) {
+ if (isPreventIntentRedirectEnabled()) {
if (mCreatorTokenInfo == null) {
out.writeInt(0);
} else {
@@ -12398,7 +12414,7 @@
mOriginalIntent = new Intent(in);
}
- if (preventIntentRedirect()) {
+ if (isPreventIntentRedirectEnabled()) {
if (in.readInt() != 0) {
mCreatorTokenInfo = new CreatorTokenInfo();
mCreatorTokenInfo.mCreatorToken = in.readStrongBinder();
diff --git a/core/java/android/security/responsible_apis_flags.aconfig b/core/java/android/security/responsible_apis_flags.aconfig
index 5457bbe..3055a3f 100644
--- a/core/java/android/security/responsible_apis_flags.aconfig
+++ b/core/java/android/security/responsible_apis_flags.aconfig
@@ -70,4 +70,11 @@
description: "Prevent intent redirect attacks"
bug: "361143368"
is_fixed_read_only: true
+}
+
+flag {
+ name: "prevent_intent_redirect_abort_or_throw_exception"
+ namespace: "responsible_apis"
+ description: "Prevent intent redirect attacks by aborting or throwing security exception"
+ bug: "361143368"
}
\ No newline at end of file
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 746c55f..4b8f1c8 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -60,6 +60,7 @@
import static android.app.ProcessMemoryState.HOSTING_COMPONENT_TYPE_INSTRUMENTATION;
import static android.app.ProcessMemoryState.HOSTING_COMPONENT_TYPE_PERSISTENT;
import static android.app.ProcessMemoryState.HOSTING_COMPONENT_TYPE_SYSTEM;
+import static android.content.Intent.isPreventIntentRedirectEnabled;
import static android.content.pm.ApplicationInfo.HIDDEN_API_ENFORCEMENT_DEFAULT;
import static android.content.pm.PackageManager.GET_SHARED_LIBRARY_FILES;
import static android.content.pm.PackageManager.MATCH_ALL;
@@ -130,7 +131,6 @@
import static android.provider.Settings.Global.ALWAYS_FINISH_ACTIVITIES;
import static android.provider.Settings.Global.DEBUG_APP;
import static android.provider.Settings.Global.WAIT_FOR_DEBUGGER;
-import static android.security.Flags.preventIntentRedirect;
import static android.util.FeatureFlagUtils.SETTINGS_ENABLE_MONITOR_PHANTOM_PROCS;
import static android.view.Display.INVALID_DISPLAY;
@@ -19267,7 +19267,7 @@
* @hide
*/
public void addCreatorToken(@Nullable Intent intent, String creatorPackage) {
- if (!preventIntentRedirect()) return;
+ if (!isPreventIntentRedirectEnabled()) return;
if (intent == null || intent.getExtraIntentKeys() == null) return;
for (String key : intent.getExtraIntentKeys()) {
@@ -19278,7 +19278,9 @@
+ "} does not correspond to an intent in the extra bundle.");
continue;
}
- Slog.wtf(TAG, "A creator token is added to an intent.");
+ Slog.wtf(TAG,
+ "A creator token is added to an intent. creatorPackage: " + creatorPackage
+ + "; intent: " + intent);
IBinder creatorToken = createIntentCreatorToken(extraIntent, creatorPackage);
if (creatorToken != null) {
extraIntent.setCreatorToken(creatorToken);
diff --git a/services/core/java/com/android/server/wm/ActivityStartController.java b/services/core/java/com/android/server/wm/ActivityStartController.java
index c1f5a27..a2c2dfc 100644
--- a/services/core/java/com/android/server/wm/ActivityStartController.java
+++ b/services/core/java/com/android/server/wm/ActivityStartController.java
@@ -478,10 +478,10 @@
intentGrants.merge(creatorIntentGrants);
}
} catch (SecurityException securityException) {
- ActivityStarter.logForIntentRedirect(
+ ActivityStarter.logAndThrowExceptionForIntentRedirect(
"Creator URI Grant Caused Exception.", intent, creatorUid,
- creatorPackage, filterCallingUid, callingPackage);
- // TODO b/368559093 - rethrow the securityException.
+ creatorPackage, filterCallingUid, callingPackage,
+ securityException);
}
}
if ((aInfo.applicationInfo.privateFlags
diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java
index 5d3ae54..2c4179f 100644
--- a/services/core/java/com/android/server/wm/ActivityStarter.java
+++ b/services/core/java/com/android/server/wm/ActivityStarter.java
@@ -54,6 +54,7 @@
import static android.content.pm.ActivityInfo.LAUNCH_SINGLE_TOP;
import static android.content.pm.ActivityInfo.launchModeToString;
import static android.os.Process.INVALID_UID;
+import static android.security.Flags.preventIntentRedirectAbortOrThrowException;
import static android.view.Display.DEFAULT_DISPLAY;
import static android.view.WindowManager.TRANSIT_NONE;
import static android.view.WindowManager.TRANSIT_OPEN;
@@ -100,9 +101,11 @@
import android.app.ProfilerInfo;
import android.app.WaitResult;
import android.app.WindowConfiguration;
+import android.app.compat.CompatChanges;
import android.compat.annotation.ChangeId;
import android.compat.annotation.Disabled;
import android.compat.annotation.EnabledSince;
+import android.compat.annotation.Overridable;
import android.content.IIntentSender;
import android.content.Intent;
import android.content.IntentSender;
@@ -188,6 +191,10 @@
@Disabled
static final long ASM_RESTRICTIONS = 230590090L;
+ @ChangeId
+ @Overridable
+ private static final long ENABLE_PREVENT_INTENT_REDIRECT_TAKE_ACTION = 29623414L;
+
private final ActivityTaskManagerService mService;
private final RootWindowContainer mRootWindowContainer;
private final ActivityTaskSupervisor mSupervisor;
@@ -608,11 +615,10 @@
// Check if the Intent was redirected
if ((intent.getExtendedFlags() & Intent.EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN)
!= 0) {
- ActivityStarter.logForIntentRedirect(
+ ActivityStarter.logAndThrowExceptionForIntentRedirect(
"Unparceled intent does not have a creator token set.", intent,
- intentCreatorUid,
- intentCreatorPackage, resolvedCallingUid, resolvedCallingPackage);
- // TODO b/368559093 - eventually ramp up to throw SecurityException
+ intentCreatorUid, intentCreatorPackage, resolvedCallingUid,
+ resolvedCallingPackage, null);
}
if (IntentCreatorToken.isValid(intent)) {
IntentCreatorToken creatorToken = (IntentCreatorToken) intent.getCreatorToken();
@@ -645,11 +651,10 @@
intentGrants.merge(creatorIntentGrants);
}
} catch (SecurityException securityException) {
- ActivityStarter.logForIntentRedirect(
+ ActivityStarter.logAndThrowExceptionForIntentRedirect(
"Creator URI Grant Caused Exception.", intent, intentCreatorUid,
intentCreatorPackage, resolvedCallingUid,
- resolvedCallingPackage);
- // TODO b/368559093 - rethrow the securityException.
+ resolvedCallingPackage, securityException);
}
}
} else {
@@ -670,11 +675,10 @@
intentGrants.merge(creatorIntentGrants);
}
} catch (SecurityException securityException) {
- ActivityStarter.logForIntentRedirect(
+ ActivityStarter.logAndThrowExceptionForIntentRedirect(
"Creator URI Grant Caused Exception.", intent, intentCreatorUid,
intentCreatorPackage, resolvedCallingUid,
- resolvedCallingPackage);
- // TODO b/368559093 - rethrow the securityException.
+ resolvedCallingPackage, securityException);
}
}
}
@@ -1045,7 +1049,7 @@
int callingUid = request.callingUid;
int intentCreatorUid = request.intentCreatorUid;
String intentCreatorPackage = request.intentCreatorPackage;
- String intentCallingPackage = request.callingPackage;
+ String callingPackage = request.callingPackage;
String callingFeatureId = request.callingFeatureId;
final int realCallingPid = request.realCallingPid;
final int realCallingUid = request.realCallingUid;
@@ -1130,7 +1134,7 @@
// launched in the app flow to redirect to an activity picked by the user, where
// we want the final activity to consider it to have been launched by the
// previous app activity.
- intentCallingPackage = sourceRecord.launchedFromPackage;
+ callingPackage = sourceRecord.launchedFromPackage;
callingFeatureId = sourceRecord.launchedFromFeatureId;
}
}
@@ -1152,7 +1156,7 @@
if (packageArchiver.isIntentResolvedToArchivedApp(intent, mRequest.userId)) {
err = packageArchiver
.requestUnarchiveOnActivityStart(
- intent, intentCallingPackage, mRequest.userId, realCallingUid);
+ intent, callingPackage, mRequest.userId, realCallingUid);
}
}
}
@@ -1211,7 +1215,7 @@
boolean abort;
try {
abort = !mSupervisor.checkStartAnyActivityPermission(intent, aInfo, resultWho,
- requestCode, callingPid, callingUid, intentCallingPackage, callingFeatureId,
+ requestCode, callingPid, callingUid, callingPackage, callingFeatureId,
request.ignoreTargetSecurity, inTask != null, callerApp, resultRecord,
resultRootTask);
} catch (SecurityException e) {
@@ -1239,7 +1243,7 @@
abort |= !mService.mIntentFirewall.checkStartActivity(intent, callingUid,
callingPid, resolvedType, aInfo.applicationInfo);
abort |= !mService.getPermissionPolicyInternal().checkStartActivity(intent, callingUid,
- intentCallingPackage);
+ callingPackage);
if (intentCreatorUid != Request.DEFAULT_INTENT_CREATOR_UID) {
try {
@@ -1247,36 +1251,29 @@
requestCode, 0, intentCreatorUid, intentCreatorPackage, "",
request.ignoreTargetSecurity, inTask != null, null, resultRecord,
resultRootTask)) {
- logForIntentRedirect("Creator checkStartAnyActivityPermission Caused abortion.",
+ abort = logAndAbortForIntentRedirect(
+ "Creator checkStartAnyActivityPermission Caused abortion.",
intent, intentCreatorUid, intentCreatorPackage, callingUid,
- intentCallingPackage);
- // TODO b/368559093 - set abort to true.
- // abort = true;
+ callingPackage);
}
} catch (SecurityException e) {
- logForIntentRedirect("Creator checkStartAnyActivityPermission Caused Exception.",
- intent, intentCreatorUid, intentCreatorPackage, callingUid,
- intentCallingPackage);
- // TODO b/368559093 - rethrow the exception.
- //throw e;
+ logAndThrowExceptionForIntentRedirect(
+ "Creator checkStartAnyActivityPermission Caused Exception.",
+ intent, intentCreatorUid, intentCreatorPackage, callingUid, callingPackage,
+ e);
}
- if (!mService.mIntentFirewall.checkStartActivity(intent, intentCreatorUid, 0,
- resolvedType, aInfo.applicationInfo)) {
- logForIntentRedirect("Creator IntentFirewall.checkStartActivity Caused abortion.",
- intent, intentCreatorUid, intentCreatorPackage, callingUid,
- intentCallingPackage);
- // TODO b/368559093 - set abort to true.
- // abort = true;
+ if (!mService.mIntentFirewall.checkStartActivity(intent, intentCreatorUid,
+ 0, resolvedType, aInfo.applicationInfo)) {
+ abort = logAndAbortForIntentRedirect(
+ "Creator IntentFirewall.checkStartActivity Caused abortion.",
+ intent, intentCreatorUid, intentCreatorPackage, callingUid, callingPackage);
}
- if (!mService.getPermissionPolicyInternal().checkStartActivity(intent, intentCreatorUid,
- intentCreatorPackage)) {
- logForIntentRedirect(
+ if (!mService.getPermissionPolicyInternal().checkStartActivity(intent,
+ intentCreatorUid, intentCreatorPackage)) {
+ abort = logAndAbortForIntentRedirect(
"Creator PermissionPolicyService.checkStartActivity Caused abortion.",
- intent, intentCreatorUid, intentCreatorPackage, callingUid,
- intentCallingPackage);
- // TODO b/368559093 - set abort to true.
- // abort = true;
+ intent, intentCreatorUid, intentCreatorPackage, callingUid, callingPackage);
}
intent.removeCreatorTokenInfo();
}
@@ -1296,7 +1293,7 @@
balController.checkBackgroundActivityStart(
callingUid,
callingPid,
- intentCallingPackage,
+ callingPackage,
realCallingUid,
realCallingPid,
callerApp,
@@ -1317,7 +1314,7 @@
if (request.allowPendingRemoteAnimationRegistryLookup) {
checkedOptions = mService.getActivityStartController()
.getPendingRemoteAnimationRegistry()
- .overrideOptionsIfNeeded(intentCallingPackage, checkedOptions);
+ .overrideOptionsIfNeeded(callingPackage, checkedOptions);
}
if (mService.mController != null) {
try {
@@ -1334,7 +1331,7 @@
final TaskDisplayArea suggestedLaunchDisplayArea =
computeSuggestedLaunchDisplayArea(inTask, sourceRecord, checkedOptions);
mInterceptor.setStates(userId, realCallingPid, realCallingUid, startFlags,
- intentCallingPackage,
+ callingPackage,
callingFeatureId);
if (mInterceptor.intercept(intent, rInfo, aInfo, resolvedType, inTask, inTaskFragment,
callingPid, callingUid, checkedOptions, suggestedLaunchDisplayArea)) {
@@ -1372,7 +1369,7 @@
if (mService.getPackageManagerInternalLocked().isPermissionsReviewRequired(
aInfo.packageName, userId)) {
final IIntentSender target = mService.getIntentSenderLocked(
- ActivityManager.INTENT_SENDER_ACTIVITY, intentCallingPackage,
+ ActivityManager.INTENT_SENDER_ACTIVITY, callingPackage,
callingFeatureId,
callingUid, userId, null, null, 0, new Intent[]{intent},
new String[]{resolvedType}, PendingIntent.FLAG_CANCEL_CURRENT
@@ -1436,7 +1433,7 @@
// app [on install success].
if (rInfo != null && rInfo.auxiliaryInfo != null) {
intent = createLaunchIntent(rInfo.auxiliaryInfo, request.ephemeralIntent,
- intentCallingPackage, callingFeatureId, verificationBundle, resolvedType,
+ callingPackage, callingFeatureId, verificationBundle, resolvedType,
userId);
resolvedType = null;
callingUid = realCallingUid;
@@ -1460,7 +1457,7 @@
.setCaller(callerApp)
.setLaunchedFromPid(callingPid)
.setLaunchedFromUid(callingUid)
- .setLaunchedFromPackage(intentCallingPackage)
+ .setLaunchedFromPackage(callingPackage)
.setLaunchedFromFeature(callingFeatureId)
.setIntent(intent)
.setResolvedType(resolvedType)
@@ -3588,16 +3585,32 @@
pw.println(mInTaskFragment);
}
- static void logForIntentRedirect(String message, Intent intent, int intentCreatorUid,
- String intentCreatorPackage, int callingUid, String callingPackage) {
+ static void logAndThrowExceptionForIntentRedirect(@NonNull String message,
+ @NonNull Intent intent, int intentCreatorUid, @Nullable String intentCreatorPackage,
+ int callingUid, @Nullable String callingPackage,
+ @Nullable SecurityException originalException) {
String msg = getIntentRedirectPreventedLogMessage(message, intent, intentCreatorUid,
intentCreatorPackage, callingUid, callingPackage);
Slog.wtf(TAG, msg);
+ if (preventIntentRedirectAbortOrThrowException() && CompatChanges.isChangeEnabled(
+ ENABLE_PREVENT_INTENT_REDIRECT_TAKE_ACTION, callingUid)) {
+ throw new SecurityException(msg, originalException);
+ }
}
- private static String getIntentRedirectPreventedLogMessage(String message, Intent intent,
- int intentCreatorUid, String intentCreatorPackage, int callingUid,
- String callingPackage) {
+ private static boolean logAndAbortForIntentRedirect(@NonNull String message,
+ @NonNull Intent intent, int intentCreatorUid, @Nullable String intentCreatorPackage,
+ int callingUid, @Nullable String callingPackage) {
+ String msg = getIntentRedirectPreventedLogMessage(message, intent, intentCreatorUid,
+ intentCreatorPackage, callingUid, callingPackage);
+ Slog.wtf(TAG, msg);
+ return preventIntentRedirectAbortOrThrowException() && CompatChanges.isChangeEnabled(
+ ENABLE_PREVENT_INTENT_REDIRECT_TAKE_ACTION, callingUid);
+ }
+
+ private static String getIntentRedirectPreventedLogMessage(@NonNull String message,
+ @NonNull Intent intent, int intentCreatorUid, @Nullable String intentCreatorPackage,
+ int callingUid, @Nullable String callingPackage) {
return "[IntentRedirect]" + message + " intentCreatorUid: " + intentCreatorUid
+ "; intentCreatorPackage: " + intentCreatorPackage + "; callingUid: " + callingUid
+ "; callingPackage: " + callingPackage + "; intent: " + intent;